Re enable move tab left / right via keyboard shortcuts
ClosedPublic

Authored by tcanabrava on Mar 29 2019, 4:50 PM.

Details

Summary

Re-enable Move tab to left / right via keyboard shortcuts

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
tcanabrava created this revision.Mar 29 2019, 4:50 PM
Restricted Application added a project: Konsole. · View Herald TranscriptMar 29 2019, 4:50 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
tcanabrava requested review of this revision.Mar 29 2019, 4:50 PM
tcanabrava edited the summary of this revision. (Show Details)Mar 29 2019, 4:51 PM
tcanabrava added reviewers: Konsole, hindenburg.
tcanabrava updated this revision to Diff 55038.Mar 29 2019, 4:51 PM
  • Remove empty line

This doesn't seem to work at all... tested w/ just 2 tabs

Just tried this patch and it seems to work just fine for me.

src/ViewManager.cpp
204

"Focus Right Terminal" would be more consistent

247

restore-other-terminal should be something like "move-tab-right"

254

Same here. Also if multiple actions have the same name you can only configure the last one in the shortcut config dialog.

OK I'll try again - not sure why it didn't work for me the first time.

The QAction names need to stay the same if possible, otherwise previous shortcuts don't work. If they are changed, I'm not sure there's a way to automatically update them.

~/local/share/kxmlgui5/konsole/konsoleui.rc

<Action name="move-view-right" shortcut="Ctrl+Alt+["/>
<Action name="restore-other-terminals" shortcut="Ctrl+Alt+]"/>

previous version

<Action shortcut="Ctrl+Alt+]" name="move-view-right"/>
<Action shortcut="Ctrl+Alt+[" name="move-view-left"/>
tcanabrava updated this revision to Diff 55822.Apr 9 2019, 2:15 PM
  • Fix action identifiers

this should fix most of the issues. I have another patch to go related to actions, but this one needs to land first.

The one issue I have, if you open just one tab and press the move left/right I get C for 'move right' and D for 'move left" echoed on the terminal. They go away when I open another terminal and don't appear to return even if I close all but one.

Don't worry too much about the minor nitpicks.

src/ViewContainer.cpp
567

empty space at end - minor

582

minor nitpick that doesn't have to fix now - make sure there are newlines at end of files.

The one issue I have, if you open just one tab and press the move left/right I get C for 'move right' and D for 'move left" echoed on the terminal. They go away when I open another terminal and don't appear to return even if I close all but one.

ON master w/o this patch, I still get the CD echoing on all the tabs when I try the shortcuts. So maybe it is something else. Is anyone else seeing this issue?

The one issue I have, if you open just one tab and press the move left/right I get C for 'move right' and D for 'move left" echoed on the terminal. They go away when I open another terminal and don't appear to return even if I close all but one.

ON master w/o this patch, I still get the CD echoing on all the tabs when I try the shortcuts. So maybe it is something else. Is anyone else seeing this issue?

Probably it's the fact that I'm enabling the tab shortcut only if there's more than one tab. when it's disabling, the shortcut will not go thru the action event, and thus will be send to the terminal emulator.
that's not relevant for this patch, though. then I ask you to accept this one and I'll look into the bug in master.

hindenburg accepted this revision.Apr 10 2019, 1:47 PM

Sounds good

This revision is now accepted and ready to land.Apr 10 2019, 1:47 PM
This revision was automatically updated to reflect the committed changes.
dfaure added a subscriber: dfaure.Sep 6 2019, 2:14 PM
dfaure added inline comments.
src/ViewManager.cpp
248

For many years, moving tabs in konsole was done with Ctrl+Shift+Left/Right.
Any reason why this changed to Ctrl+Alt+Left/Right? I had to read the code to find out why this was "broken"...

hindenburg added inline comments.Sep 9 2019, 2:06 PM
src/ViewManager.cpp
248

I didn't catch the change; Tomaz, was this a mistake?

I needed a new accelerator for focus split right left top bottom, I talked
to David this weekend and we agreed to restore the old accelerator for move
tab.

Em seg, 9 de set de 2019 às 16:06, Kurt Hindenburg <
noreply@phabricator.kde.org> escreveu:

hindenburg added inline comments. View Revision
https://phabricator.kde.org/D20115
*INLINE COMMENTS*
View Inline https://phabricator.kde.org/D20115#inline-134343dfaure
wrote in ViewManager.cpp:248

For many years, moving tabs in konsole was done with Ctrl+Shift+Left/Right.
Any reason why this changed to Ctrl+Alt+Left/Right? I had to read the code
to find out why this was "broken"...

I didn't catch the change; Tomaz, was this a mistake?

*REPOSITORY*
R319 Konsole

*REVISION DETAIL*
https://phabricator.kde.org/D20115

*To: *tcanabrava, Konsole, hindenburg
*Cc: *dfaure, mschiller, konsole-devel, fbampaloukas, thsurrel, ngraham,
maximilianocuria, hindenburg