This fixes the issue when bookmark search is started with 's' or 'j'.
Previously these letters were accelerators coming from the Go global menu,
which prevented from searching bookmarks started from the letters.
Now, the bookmark menu has a dedicated actions without the accelerators.
Details
- Reviewers
martinkostolny rade - Group Reviewers
Krusader - Commits
- R167:6fd0ed74d8d3: Made order of jump-back actions the same as in Go menu
R167:f515bfa6b40a: Fixed jump-back actions in bookmark menu, improved separators
R167:16befc43215b: Fixed shortcuts for jump-back actions in bookmark menu
R167:2841d0f45dc6: Fixed bookmark search when started with S or J, improved menu style
Confirm that jump back actions are working from both Go menu and bookmark menu.
Check if designated shortcuts are working.
Diff Detail
- Repository
- R167 Krusader
- Branch
- jump-back-issue
- Lint
No Linters Available - Unit
No Unit Test Coverage
Read through the code and it looks good.
One 'problem' though is that the shortcut 'Ctrl+j' and 'Ctrl+shift+j' used to be displayed next to jump back point and set jump back point the same way they still are in the go menu. That should probably be fixed.
krusader/BookMan/krbookmark.cpp | ||
---|---|---|
127 | I suspect this is where you are overwriting the 'Ctrl+j' part of the name. |
Agreed with Rade about the missing displayed shortcuts. Otherwise it works nicely, thanks!
I'm intentionally not copying the shortcut - in case I do copy and issue the shortcut, krusader claims there are ambiguous shortcuts because there are two actions with the same shortcut. Although the shortcuts for the two actions are not shown on bookmark menu, they still work because they are assigned to source actions (from Go menu).
krusader/BookMan/krbookmark.cpp | ||
---|---|---|
127 | Earlier on the line 122 I put a comment: shortcut is not copied as it will introduce ambiguity. removeAcceleratorMarker only removes accelerator key marked with &. |
You could add back the the shortcut text only using
´´´
QString shortcutText = sourceAction->shortcut().toString();
´´´
But then the text obviously doesn't get right justified like the other shortcut text. It cold be right justified manually when we detect the show event and there for know the size of the menu. But that's hacky, and doesn't deal with if the menu gets resized etc.
The better option then too me would seem to be to not copy the sourceaction at all. Just add it to the menu like we used to do. Split the begining if in eventfilter that detects the show and close event into two ifs one for show and one for close. In the if that handles show, right after we do reset, just strip the accelerator from set jump back and jump back, and put the original strings in _quickSearchOriginalActionTitles. Once the menu closes they'll get their accelerators back again.
- Fixed shortcuts for jump-back actions in bookmark menu
- Made order of jump-back actions the same as in Go menu
I have found a way to assign shortcuts without introducing ambiguity. Please test and accept if it works.