search

tibirna / qgit

Official git repository for QGit.
Other
174 stars 68 forks source link

Fix broken keybindings (i.e. n, k) and adjust Readme to current keybindings #5

Closed ursjoss closed 7 years ago

ursjoss commented 7 years ago

Hi

I noticed that in qgit-2.7 some of the key bindings did not work anymore. Especially n/k (to move down one revision) not working was quite annoing and prevented me from upgrading.

I also adjusted the readme to reflect the current keybindings for going to revisions page (r -> CTRL-r) and to the patch page (p -> CTRL-p). There might be different mismatches, but most keys I did try with the patched versions were doing what I expected.

Thanks and cheers Urs

tibirna commented 7 years ago

Hello Urs

Thanks for the patches. The 792c94f is OK. Thanks a lot for catching that. I don't understand, though, why you changed the documentation in 52986cf. The 'r' and 'p' shortcuts still work as intended, without the CTRL modifier.

Thanks Cristian

ursjoss commented 7 years ago

Hi Cristian

Thanks for your quick response. I double-checked qgit on my machine (;-) ) and in fact, the keys are remapped to ctrl-r/ctrl-t somehow. See screenshot ). But I can see in the code that it's actually using r/p by default. Not sure how and by what component this remapping is done but maybe it's KDE specific.

Anyhow, in order to get the actual fix merged, I reverted commit 52986cfe901b1993dd91c0fecb74e1948c63739a .

Thx and cheers Urs

tibirna commented 7 years ago

Hey Urs

There are both explicit accelerators (in the code you patched) and QAction objects, in which the Ctrl+* key combinations are defined. I don't think this is harmful.

I'll pull your request ASAP.

Thanks again Cristian

On 28 May 2017 at 23:57, Urs Joss notifications@github.com wrote:

Hi Cristian

Thanks for your quick response. I double-checked qgit on my machine (;-) ) and in fact, the keys are remapped to ctrl-r/ctrl-t somehow. See screenshot https://github.com/ursjoss/qgit/blob/tb_screenshot/tmp/qgit_keymappings.png ). Not sure how and by what component this remapping is done but maybe it's KDE specific.

Anyhow, in order to get the actual fix merged, I reverted commit 52986cf https://github.com/tibirna/qgit/commit/52986cfe901b1993dd91c0fecb74e1948c63739a .

Thx and cheers Urs

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/tibirna/qgit/pull/5#issuecomment-304567359, or mute the thread https://github.com/notifications/unsubscribe-auth/ABLrDh3PuWgXqRLhlArtBzCday_aBRCrks5r-kImgaJpZM4NkJcj .

ursjoss commented 7 years ago

Thanks Cristian.

Cheers, Urs