Profile shortcuts switch profile instead of opening new tab
ClosedPublic

Authored by thsurrel on Dec 5 2018, 10:08 PM.

Details

Summary

This is a proposal to modify the profile shortcuts behaviour:
instead of opening a new tab with the corresponding profile
they now switch the profile of the current terminal display.

FEATURE: 319926

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.
thsurrel created this revision.Dec 5 2018, 10:08 PM
Restricted Application added a project: Konsole. · View Herald TranscriptDec 5 2018, 10:08 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
thsurrel requested review of this revision.Dec 5 2018, 10:08 PM

I don't really know how to test this in the embedded terminal in Kate for example. Could someone provide some guidance ?

I don't really know how to test this in the embedded terminal in Kate for example. Could someone provide some guidance ?

You can try to use the demo_konsolepart in src/tests/demo_konsolepart/ - or try to copy the home built konsolepart.so to a location the other apps will find them. (Depending on OS/distro exampe: /usr/lib/x86_64-linux-gnu/qt5/plugins/). If you have your distros konsole/konsolepart installed, it might cause issues.

I don't think I agree that using a profile shortcut should switch the current tab to that profile. What would be rationale for this change?

I don't think I agree that using a profile shortcut should switch the current tab to that profile. What would be rationale for this change?

I would say "Expectations", when I go to the profile I expect it to switch the currently focused terminal to that profile, if the button was labelled "new terminal with profile" then I would be against that change. I belive both variants could coexist.

thsurrel added a comment.EditedDec 13 2018, 12:54 PM

Two other things:

  • there is already a shortcut to open a new tab (Ctrl-Shift-t), but nothing to change the profile of the current terminal. With this patch you can do everything from the keyboard.
  • as suggested in FEATURE: 319926, some users (me included) would like to be able to quickly switch temporarily to another profile and back.

I would say "Expectations", when I go to the profile I expect it to switch the currently focused terminal to that profile,

+1

OK, I never really use switch unless I'm testing something - I'll look at the code

Can you look the change I requested - a quick coding here seems to work w/ the changes.

src/SessionController.cpp
226

This isn't good; instead of putting getProfileList() in ViewManager, put it in ProfileManager (without QObject) and then do '_profileList = ProfileManager::instance()->getProfileList();
'

thsurrel updated this revision to Diff 48419.Dec 30 2018, 9:24 PM

Fix requested by hindenburg
Thank you very much for the review

hindenburg accepted this revision.Dec 30 2018, 11:07 PM

This is very nice - thanks for working on this and staying with it. Good catch with the read only code.

This revision is now accepted and ready to land.Dec 30 2018, 11:07 PM
This revision was automatically updated to reflect the committed changes.