Add shortcut to move a line up and down #337
Conversation
|
Maybe implement it as QAction instead? This way you will be able to show it in Edit menu and right-click menu for increased visibility. |
|
@mitya57 Done, I also added the actions in the tool bar |
xgouchet
force-pushed
the
alt_move_line
branch
from
df36715
to
a6e7d22
Compare
ReText/editor.py
Outdated
| @@ -197,6 +197,9 @@ def contextMenuEvent(self, event): | |||
| actions = [self.parent.act(sug, trig=self.fixWord(sug)) for sug in suggestions] | |||
| menu = self.createStandardContextMenu() | |||
| menu.insertSeparator(menu.actions()[0]) | |||
| menu.insertAction(menu.actions()[0], self.parent.actionMoveDown) | |||
| menu.insertAction(menu.actions()[0], self.parent.actionMoveUp) | |||
| menu.insertSeparator(menu.actions()[0]) | |||
There was a problem hiding this comment.
This method returns if no text is selected, see above. Please move this code before line 188.
I would also use addAction/addSeparator instead of insertAction/insertSeparator.
There was a problem hiding this comment.
I used insert* instead of add* to match the other lines in this method. Should we use add* for all of them ?
Also I can't move those lines up as the menu is not created yet. Or we should not use the default menu creation at all in this case.
There was a problem hiding this comment.
insert*(menu.actions()[0], ...) adds an action top top, add*(...) adds to bottom. In my opinion spell checker actions are popular and should be on top, these ones are less popular and can be on bottom.
What do you mean by “not use the default menu creation at all”?
I think the solution is like this:
- Always create the menu on top of this function, always append the move up/down actions;
- Replace
return QTextEdit.contextMenuEvent(self, event)withmenu.exec(event.globalPos()).
There was a problem hiding this comment.
Done. I kinda rewrote the method to add the action even if we don't need the spellcheck, while keeping the spellcheck options on top, and the move at the bottom
ReText/window.py
Outdated
| @@ -336,6 +343,9 @@ def __init__(self, parent=None): | |||
| self.editBar.addAction(self.actionCopy) | |||
| self.editBar.addAction(self.actionPaste) | |||
| self.editBar.addSeparator() | |||
| self.editBar.addAction(self.actionMoveUp) | |||
| self.editBar.addAction(self.actionMoveDown) | |||
| self.editBar.addSeparator() | |||
There was a problem hiding this comment.
Is it really such a common action that it needs to be on the toolbar?
There was a problem hiding this comment.
Not really, I can remove it if you prefer
ReText/editor.py
Outdated
| elif key == Qt.Key_Down and event.modifiers() & Qt.AltModifier: | ||
| self.moveLineDown() | ||
| elif key == Qt.Key_Up and event.modifiers() & Qt.AltModifier: | ||
| self.moveLineUp() |
There was a problem hiding this comment.
These lines are no longer needed, the shortcut is handled by QAction.
xgouchet
force-pushed
the
alt_move_line
branch
2 times, most recently
from
662e90f
to
0815042
Compare
|
By the way I have invited you to @retext-project, but please still make pull requests for non-trivial changes. |
|
Wow thanks man. And don't worry, I think I'll still make pull request anyway to get a second pair of eyes on my code ! |
xgouchet
force-pushed
the
alt_move_line
branch
from
0815042
to
32547a0
Compare
There was a problem hiding this comment.
You broke something :(
Now if I click on a spell check suggestion, the fixed word is inserted where the cursor was, not where right click was performed.
The previous behavior selected the wrong word on right click, and selecting a fixed word replaced that selection.
xgouchet
force-pushed
the
alt_move_line
branch
from
32547a0
to
9b378bd
Compare
|
Indeed, this should be fixed now |
There was a problem hiding this comment.
Hmm, now it selects the word and shows spell check suggestions even if the word is correct.
Also I noticed that if you right click outside a word (i.e. on a space between words) and select “Move line down” then it moves the wrong line down. Not one on which you clicked. Maybe unconditionally place the cursor to the click place before showing the menu?
Pressing Alt+Arrow Up / Alt+Arrow Down will move the current line up/down. This is a nice feature to move lines without having to do a Select-Cut-Move-Paste manipulation.
xgouchet
force-pushed
the
alt_move_line
branch
from
9b378bd
to
69bd70d
Compare
|
Done |
|
Don't know if that was intended or not but you cannot move multiple lines (only one). I am commenting here since I don't know if this would be a new feature (if yes I will add this as a new issue) or if this is just a bug. |
|
I don't think moving multiple lines is a common task. And there is always Cut+Paste if you want that. |
Pressing Alt+Arrow Up / Alt+Arrow Down will move the current line
up/down. This is a nice feature to move lines without having to do a
Select-Cut-Move-Paste manipulation.