-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PR: Allow undo/redo on Source > Fix indentation #5657
Conversation
# We do the following rather than using self.setPlainText | ||
# to benefit from QTextEdit's undo/redo feature. | ||
self.selectAll() | ||
self.insertPlainText(text_after) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice catch!
@csabella thanks a lot for working on this. LGTM! |
@rlaverde want to give it another pass? |
@@ -907,12 +907,14 @@ def remove_trailing_spaces(self): | |||
cursor.endEditBlock() | |||
|
|||
def fix_indentation(self): | |||
"""Replace tabs by spaces""" | |||
"""Replace tabs by 4 spaces.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csabella isn't this fix dependent on the preference setting for what a tab should represent? Python is 4 but a user could change this to 3 or 2 if they follow google's ugly style guide.
Not sure this is the case but if it is then.
"""Replace tabs by number of spaces (for tabs) defined in the preferences."""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, @goanpeca is right about this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't use the settings now. It's hard-coded as 4, which is why I made the comment. It's in sourcecode/utils.py
.
def fix_indentation(text):
"""Replace tabs by spaces"""
return text.replace('\t', ' '*4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@csabella so then we need to fix fix_indentation perhaps?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@goanpeca Sure. Should it be in this PR or a separate issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I fixed something regarding the use of preferences but in the comment/uncomment functionality in #5470, is that related with this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's similar, but not the same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, let's leave this for another PR then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opened issue #5678 for the number of spaces.
@@ -84,5 +84,35 @@ def test_selection_indent(code_editor_indent_bot): | |||
) | |||
|
|||
|
|||
def test_fix_indentation(code_editor_indent_bot): | |||
"""Test fix_indentation() method.""" | |||
ed, qtbot = code_editor_indent_bot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor nitpick, but maybe editor is a better word :-), we are not saving that much space bellow anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Fixes issue #1754.
Fix indentation
converts all the tabs in a document to 4 spaces. Allow undo/redo on this operation.