Re: Review Request 124213: Add standard shortcut for new tab action

2015-07-03 Thread Simon Persson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/
---

(Updated July 4, 2015, 1:04 a.m.)


Review request for KDE Frameworks.


Changes
---

Remove shortcut ctrl+shift+N, make ctrl+T the primary and only shortcut.


Repository: kconfig


Description
---

QKeySequence already has AddTab as a standard shortcut, but it was missing in 
KStandardShortcut. Main problem with this is that users could not configure the 
shortcut globally. This is of course only a first step, applications would have 
to be updated to use this also.
Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running on 
KDE, this patch just goes with that as the chosen default.
Also fixes documentation error for tabNext and tabPrev that I happened to 
notice.


Diffs (updated)
-

  src/gui/kstandardshortcut.h 5bb07fb 
  src/gui/kstandardshortcut.cpp 8400737 

Diff: https://git.reviewboard.kde.org/r/124213/diff/


Testing
---

Compiles, test suite ok.


Thanks,

Simon Persson

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 124213: Add standard shortcut for new tab action

2015-07-01 Thread Simon Persson


 On June 30, 2015, 12:07 p.m., Kai Uwe Broulik wrote:
  +1 for Ctrl+T but not sure about Ctrl+Shift+N, never seen that used for 
  that purpose, just incognito mode, or new window in applications like 
  Konsole.

You can see that ctrl+. (period) is the current default in 
kstandardshortcut.cpp. There are still many applications that are not getting 
the default for next/prev tab from KStandardShortcut. Just try switching the 
global setting and go around different applications and see where the new 
shortcuts work... 

I have also never encountered ctrl+shift+N for this action... makes me wonder 
why Qt uses it on KDE (check the documentation on QKeySequence). Actually, 
since this is about standard shortcuts that are likely to be used in many 
applications the risk of assignment conflicts need to be considered. If ctrl+T 
is the only commonly used shortcut then I would think that it's better to only 
provide one default and leave ctrl+shift+N available.


- Simon


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/#review81896
---


On June 30, 2015, 9:13 a.m., Simon Persson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124213/
 ---
 
 (Updated June 30, 2015, 9:13 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 QKeySequence already has AddTab as a standard shortcut, but it was missing in 
 KStandardShortcut. Main problem with this is that users could not configure 
 the shortcut globally. This is of course only a first step, applications 
 would have to be updated to use this also.
 Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running 
 on KDE, this patch just goes with that as the chosen default.
 Also fixes documentation error for tabNext and tabPrev that I happened to 
 notice.
 
 
 Diffs
 -
 
   src/gui/kstandardshortcut.h 5bb07fb 
   src/gui/kstandardshortcut.cpp 8400737 
 
 Diff: https://git.reviewboard.kde.org/r/124213/diff/
 
 
 Testing
 ---
 
 Compiles, test suite ok.
 
 
 Thanks,
 
 Simon Persson
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 124213: Add standard shortcut for new tab action

2015-06-30 Thread David Faure


 On June 30, 2015, 12:07 p.m., Kai Uwe Broulik wrote:
  src/gui/kstandardshortcut.h, line 372
  https://git.reviewboard.kde.org/r/124213/diff/1/?file=381769#file381769line372
 
  Isn't Ctrl+Tab usually next tab?
 
 Aleix Pol Gonzalez wrote:
 Here, Konsole uses shift+arrow to change tab. Also it doesn't use Ctrl+T.

Konsole is always a bit special because it has to leave Ctrl+letter shortcuts 
available for terminal apps running within konsole.
And on the other hand it doesn't have lineedits, so it can use shift+arrow 
while apps with lineedits can't.
So Konsole is probably not a good example to use for the general case.


- David


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/#review81896
---


On June 30, 2015, 9:13 a.m., Simon Persson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124213/
 ---
 
 (Updated June 30, 2015, 9:13 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 QKeySequence already has AddTab as a standard shortcut, but it was missing in 
 KStandardShortcut. Main problem with this is that users could not configure 
 the shortcut globally. This is of course only a first step, applications 
 would have to be updated to use this also.
 Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running 
 on KDE, this patch just goes with that as the chosen default.
 Also fixes documentation error for tabNext and tabPrev that I happened to 
 notice.
 
 
 Diffs
 -
 
   src/gui/kstandardshortcut.h 5bb07fb 
   src/gui/kstandardshortcut.cpp 8400737 
 
 Diff: https://git.reviewboard.kde.org/r/124213/diff/
 
 
 Testing
 ---
 
 Compiles, test suite ok.
 
 
 Thanks,
 
 Simon Persson
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 124213: Add standard shortcut for new tab action

2015-06-30 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/#review81896
---


+1 for Ctrl+T but not sure about Ctrl+Shift+N, never seen that used for that 
purpose, just incognito mode, or new window in applications like Konsole.


src/gui/kstandardshortcut.h (line 372)
https://git.reviewboard.kde.org/r/124213/#comment56244

Isn't Ctrl+Tab usually next tab?


- Kai Uwe Broulik


On Juni 30, 2015, 9:13 vorm., Simon Persson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124213/
 ---
 
 (Updated Juni 30, 2015, 9:13 vorm.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 QKeySequence already has AddTab as a standard shortcut, but it was missing in 
 KStandardShortcut. Main problem with this is that users could not configure 
 the shortcut globally. This is of course only a first step, applications 
 would have to be updated to use this also.
 Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running 
 on KDE, this patch just goes with that as the chosen default.
 Also fixes documentation error for tabNext and tabPrev that I happened to 
 notice.
 
 
 Diffs
 -
 
   src/gui/kstandardshortcut.h 5bb07fb 
   src/gui/kstandardshortcut.cpp 8400737 
 
 Diff: https://git.reviewboard.kde.org/r/124213/diff/
 
 
 Testing
 ---
 
 Compiles, test suite ok.
 
 
 Thanks,
 
 Simon Persson
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 124213: Add standard shortcut for new tab action

2015-06-30 Thread Aleix Pol Gonzalez


 On June 30, 2015, 2:07 p.m., Kai Uwe Broulik wrote:
  src/gui/kstandardshortcut.h, line 372
  https://git.reviewboard.kde.org/r/124213/diff/1/?file=381769#file381769line372
 
  Isn't Ctrl+Tab usually next tab?

Here, Konsole uses shift+arrow to change tab. Also it doesn't use Ctrl+T.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/#review81896
---


On June 30, 2015, 11:13 a.m., Simon Persson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124213/
 ---
 
 (Updated June 30, 2015, 11:13 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 QKeySequence already has AddTab as a standard shortcut, but it was missing in 
 KStandardShortcut. Main problem with this is that users could not configure 
 the shortcut globally. This is of course only a first step, applications 
 would have to be updated to use this also.
 Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running 
 on KDE, this patch just goes with that as the chosen default.
 Also fixes documentation error for tabNext and tabPrev that I happened to 
 notice.
 
 
 Diffs
 -
 
   src/gui/kstandardshortcut.h 5bb07fb 
   src/gui/kstandardshortcut.cpp 8400737 
 
 Diff: https://git.reviewboard.kde.org/r/124213/diff/
 
 
 Testing
 ---
 
 Compiles, test suite ok.
 
 
 Thanks,
 
 Simon Persson
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel


Re: Review Request 124213: Add standard shortcut for new tab action

2015-06-30 Thread Aleix Pol Gonzalez

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124213/#review81894
---


Makes sense.

Maybe it would make sense to reconsider the shortcuts though? For example, I 
see some applications use Ctrl+T (i.e. Dolphin). What application did you have 
in mind?
Also New Tab is not really all that generic, is it?

- Aleix Pol Gonzalez


On June 30, 2015, 11:13 a.m., Simon Persson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/124213/
 ---
 
 (Updated June 30, 2015, 11:13 a.m.)
 
 
 Review request for KDE Frameworks.
 
 
 Repository: kconfig
 
 
 Description
 ---
 
 QKeySequence already has AddTab as a standard shortcut, but it was missing in 
 KStandardShortcut. Main problem with this is that users could not configure 
 the shortcut globally. This is of course only a first step, applications 
 would have to be updated to use this also.
 Since QKeySequence uses Ctrl+Shift+N and Ctrl+T for this action when running 
 on KDE, this patch just goes with that as the chosen default.
 Also fixes documentation error for tabNext and tabPrev that I happened to 
 notice.
 
 
 Diffs
 -
 
   src/gui/kstandardshortcut.h 5bb07fb 
   src/gui/kstandardshortcut.cpp 8400737 
 
 Diff: https://git.reviewboard.kde.org/r/124213/diff/
 
 
 Testing
 ---
 
 Compiles, test suite ok.
 
 
 Thanks,
 
 Simon Persson
 


___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel