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

2015-06-30 Thread Simon Persson

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

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


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)


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
> > 
> >
> > 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 David Faure


> On June 30, 2015, 12:07 p.m., Kai Uwe Broulik wrote:
> > src/gui/kstandardshortcut.h, line 372
> > 
> >
> > 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+ 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-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-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