[Patch] Add back search as you type too the bookmark menu

23 views
Skip to first unread message

ra...@paxemu.com

unread,
Feb 17, 2018, 1:11:19 PM2/17/18
to krusade...@googlegroups.com
Before the upgrade to Qt5 you could go to any bookmark in the bookmark
menu by simply typing the shortest unique first part of its name.
This functionality disappeared in the upgrade because kde frameworks 5
removed the functions setKeyboardShortcutsEnabled and
setKeyboardShortcutsExecute (see the commented out code in
krbookmarkbutton.cpp). As I could find no equivalent functions in kde
framework 5 I wrote some code to emulate their functionality instead.
My code seems to be functionally identical with the old one with two
exceptions
1)In krusader 2.4beta3 the text that matches what you have already typed
is underlined for each of the bookmarks
2)In my code '&' shortcuts (naming a bookmark '&movies' means 'm' is a
shortcut for that bookmark) have absolute priority. This means that if
you have three bookmarks named 'abc', 'abd', and, 'k&bc' and then type
'ab' it will open the 'kbc' bookmark, because of the 'b' shortcut. In
krusader 2.4beta3 once you have typed more than one character all the
shortcuts are disabled assuming none of them matched the first letter.


The below is the license for the patched that's attached too this email.
It's the same as the original file but i figured it might be worth
explicitly specifying just in case it was needed.
/*****************************************************************************
* This program is free software; you can redistribute it and/or modify
*
* it under the terms of the GNU General Public License as published by
*
* the Free Software Foundation; either version 2 of the License, or
*
* (at your option) any later version.
*
*
*
* This package is distributed in the hope that it will be useful,
*
* but WITHOUT ANY WARRANTY; without even the implied warranty of
*
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
*
* GNU General Public License for more details.
*
*
*
* You should have received a copy of the GNU General Public License
*
* along with this package; if not, write to the Free Software
*
* Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
USA *
*****************************************************************************/
0001-readd-search-as-you-type.patch

Martin Kotelnik

unread,
Feb 27, 2018, 6:01:05 PM2/27/18
to krusader-devel
Hi Rade!

Thanks a lot for your effort and code! I'm currently using it in my Krusader build and it seems to be working nicely.

Would you consider filing a proper review task in phabricator so it can be run through standard workflow?
https://phabricator.kde.org/differential/diff/create/

That way you'd then be able to commit it with your credentials which is a preferable way to land patches. Do you have write access and access to phabricator? It is also OK if you don't want to bother with that - we can of course proceed with the patch you've provided and mention you as the original author in final commit message. So please let us know!

Thanks again!
Regards,
Martin

Rade

unread,
Mar 1, 2018, 5:26:28 PM3/1/18
to krusade...@googlegroups.com

Hi Martin

Thank you for taking the time to test my patch and for being so welcoming!
I have created a phabricator account and submitted a review task. It can
be found here https://phabricator.kde.org/D10954

Regards
Rade
> --
> You received this message because you are subscribed to the Google
> Groups "krusader-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to krusader-deve...@googlegroups.com
> <mailto:krusader-deve...@googlegroups.com>.
> To post to this group, send email to krusade...@googlegroups.com
> <mailto:krusade...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/krusader-devel/eb36dcb0-4558-42b1-89ad-ff3c8a33a885%40googlegroups.com
> <https://groups.google.com/d/msgid/krusader-devel/eb36dcb0-4558-42b1-89ad-ff3c8a33a885%40googlegroups.com?utm_medium=email&utm_source=footer>.
> For more options, visit https://groups.google.com/d/optout.

Nikita Melnichenko

unread,
Mar 20, 2018, 2:47:35 AM3/20/18
to krusade...@googlegroups.com
We need to track the outstanding issues somewhere. Let's use this thread for the purpose, so the list resides in a single place. This is a summary from various CRs:

  • Accelerator behavior - Solved.
  • UI element to show what's typed already - Solved.
  • When bookmark search is started with 's' or 'j', the search is interrupted - Needs a review, D11444
  • Clear highlighting - Solved.
  • Folder-related: activation in multiple matches without breaking the search - Needs a review, D11443
  • Folder-related: swallowing of keys - Needs a review, D11443
  • Folder-related: opening of submenu resets the search, subsequent keypress search in submenu - No progress.
  • Not-found handling (search is reset in the middle) - Solved.
  • Don't search in special bookmarks - No progress.
  • quickSearchBar should be visible by default so it's discoverable and there is no visual jumping. Ideally with a placeholderText "Type to search...". - In progress
Please add items if I missed something.

Nikita Melnichenko

unread,
Mar 21, 2018, 2:01:49 AM3/21/18
to krusade...@googlegroups.com, Rade

+ Rade as I figured he may not be subscribed to the list.

Rade

unread,
Mar 21, 2018, 8:34:45 AM3/21/18
to krusade...@googlegroups.com
I am subscribed.

Is this a bug and not a feature? It seems useful to be able to search
submenus. If the concern is the accidentally opening thing then once
D11525 is merged that won't happen anymore.

Nikita Melnichenko

unread,
Mar 22, 2018, 3:26:12 AM3/22/18
to krusade...@googlegroups.com, Rade
Yes, it's only about opening submenu while we still have multiple
matches. When D11525 is merged we'll consider this issue resolved.

Nikita Melnichenko

unread,
Mar 26, 2018, 4:35:12 AM3/26/18
to krusade...@googlegroups.com

Ok, so everything from the previous list is solved now. We are almost ready to merge. Only two new issues are remaining:

  1. D11706 (the issue with jump back actions not closing bookmark menu when triggered).
  2. This is what I've found recently: quick search bar doesn't appear if you open bookmark menu in  another panel (actually it's even for another tab of the same panel as the tabs have different menus for some reason), i.e. only first tab you opened the menu will have search bar properly visible. It's probably something to do with QWidgetAction usage - I didn't have time to debug it yet. Martin, Rade - could you check, please? I'll be busy in the next few days.

Once we resolve these two and other we find (I hope we won't find any), I'll submit the CR to the final review and testing (mostly for people who hasn't participated in the intermediate reviews).

Thanks,
Nikita.

Rade

unread,
Mar 27, 2018, 6:17:27 PM3/27/18
to krusade...@googlegroups.com
I have unfortunately found another issue. Steps to reproduce

1. Use arrow keys to select a folder such as "Popular urls" (this works
if the folder was selected by searching as well e.g typing 'popu')
2. Use right arrow to enter folder
3. Use left arrow or esc to close submenu. Submenu item in main menu
remains highlighted and we receive a 'QEvent::Close' as expected
4. Type a letter too search
5. Search gets reset by two spurious QEvent::Close events

This does not seem too happen if the folder was simply highlighted but
never activated (I.e no right arrow key)

/Rade

On 26/03/18 10:35, Nikita Melnichenko wrote:
> Ok, so everything from the previous list is solved now. We are almost
> ready to merge. Only two new issues are remaining:
>
> 1. D11706 (the issue with jump back actions not closing bookmark menu
> when triggered).
> 2. This is what I've found recently: quick search bar doesn't appear if
> you open bookmark menu in another panel (actually it's even for
> another tab of the same panel as the tabs have different menus for
> some reason), i.e. only first tab you opened the menu will have
> search bar properly visible. It's probably something to do with
> QWidgetAction usage - I didn't have time to debug it yet. Martin,
> Rade - could you check, please? I'll be busy in the next few days.
>
> Once we resolve these two and other we find (I hope we won't find any),
> I'll submit the CR to the final review and testing (mostly for people
> who hasn't participated in the intermediate reviews).
>
> Thanks,
> Nikita.
>
> Nikita Melnichenko wrote:
>> We need to track the outstanding issues somewhere. Let's use this
>> thread for the purpose, so the list resides in a single place. This is
>> a summary from various CRs:
>>
>> * Accelerator behavior - Solved.
>> * UI element to show what's typed already - Solved.
>> * When bookmark search is started with 's' or 'j', the search is
>> interrupted - Needs a review, D11444
>> * Clear highlighting - Solved.
>> * Folder-related: activation in multiple matches without breaking
>> the search - Needs a review, D11443
>> * Folder-related: swallowing of keys - Needs a review, D11443
>> * Folder-related: opening of submenu resets the search, subsequent
>> keypress search in submenu - No progress.
>> * Not-found handling (search is reset in the middle) - Solved.
>> * Don't search in special bookmarks - No progress.
>> * quickSearchBar should be visible by default so it's discoverable
>> and there is no visual jumping. Ideally with a placeholderText
>> "Type to search...". - In progress
>>
>> Please add items if I missed something.
>>
>
> --
> You received this message because you are subscribed to the Google
> Groups "krusader-devel" group.
> To unsubscribe from this group and stop receiving emails from it, send
> an email to krusader-deve...@googlegroups.com
> <mailto:krusader-deve...@googlegroups.com>.
> To post to this group, send email to krusade...@googlegroups.com
> <mailto:krusade...@googlegroups.com>.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/krusader-devel/976bb8be-cc80-8230-874f-a054afc9de0d%40melnichenko.name
> <https://groups.google.com/d/msgid/krusader-devel/976bb8be-cc80-8230-874f-a054afc9de0d%40melnichenko.name?utm_medium=email&utm_source=footer>.
Reply all
Reply to author
Forward
0 new messages