D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir updated this revision to Diff 74893.
ahmadsamir added a comment.


  Rebase

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24951?vs=68769=74893

BRANCH
  arcpatch-D24951 (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24951

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp

To: ahmadsamir, dfaure, ngraham, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit R241:efa5438e39bb: [KOpenWithDialog] Automatically select the 
result if the model filter has only… (authored by ahmadsamir).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24951?vs=74893=74894

REVISION DETAIL
  https://phabricator.kde.org/D24951

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp

To: ahmadsamir, dfaure, ngraham, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[David Faure]

dfaure accepted this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  Ping.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added reviewers: Frameworks, VDG.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham, #frameworks, #vdg
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  In D24951#554516 , @dfaure wrote:
  
  > A sort model allows to define any sorting criteria you want (see 
QSortFilterProxyModel::lessThan), so this is technically possible: the criteria 
would say "if A is a match and B isn't, then A  I'm not 100% sure that things jumping around while typing is a good idea 
though.
  
  
  In this case it would have to be "if A is shorter than B"; also this is a 
tree model, we want to sort based on the leaf nodes (.desktop Name=), but not 
on parent nodes (the category names), I am not sure that is 
possible/straightforward with that item model.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  In D24951#554516 , @dfaure wrote:
  
  > A sort model allows to define any sorting criteria you want (see 
QSortFilterProxyModel::lessThan), so this is technically possible: the criteria 
would say "if A is a match and B isn't, then A  I'm not 100% sure that things jumping around while typing is a good idea 
though.
  
  
  
  
  In D24951#554516 , @dfaure wrote:
  
  > A sort model allows to define any sorting criteria you want (see 
QSortFilterProxyModel::lessThan), so this is technically possible: the criteria 
would say "if A is a match and B isn't, then Ahttps://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[David Faure]

dfaure added a comment.


  By "jumping around" I mean that even order can change, not just things being 
added/removed.
  A would be before B, and then you type something, and B jumps before A.
  Or do I misunderstand and this could never happen?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Nathaniel Graham]

ngraham added a comment.


  In D24951#554516 , @dfaure wrote:
  
  > I'm not 100% sure that things jumping around while typing is a good idea 
though.
  
  
  That's exactly what already happens though, as the list of search narrows 
itself while you type. KRunner does this too. I don't think it's a problem.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[David Faure]

dfaure added a comment.


  A sort model allows to define any sorting criteria you want (see 
QSortFilterProxyModel::lessThan), so this is technically possible: the criteria 
would say "if A is a match and B isn't, then Ahttps://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  In D24951#554485 , @ngraham wrote:
  
  > In D24951#554468 , @ahmadsamir 
wrote:
  >
  > > The user typing 'arc' then pressing Enter, and getting the file opened 
with wireshark is broken behaviour form the user's POV, that's the crux of the 
bug report in question.
  >
  >
  > If the point is to fix just this corner case, maybe we can do so by putting 
exact matches at the top of the list.
  
  
  I don't think that's possible; the proxy sort model doesn't offer a way to 
change the order of the shown results, IIUC, it just filters out/hides non 
matching items and displays what's left.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Nathaniel Graham]

ngraham added a comment.


  In D24951#554468 , @ahmadsamir 
wrote:
  
  > The user typing 'arc' then pressing Enter, and getting the file opened with 
wireshark is broken behaviour form the user's POV, that's the crux of the bug 
report in question.
  
  
  If the point is to fix just this corner case, maybe we can do so by putting 
exact matches at the top of the list.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  The user typing 'arc' then pressing Enter, and getting the file opened with 
wireshark is broken behaviour form the user's POV, that's the crux of the bug 
report in question.
  
  That combobox uses KCompletion and not having the up/down arrow keys navigate 
through the possible completions in the popup menu, when the mode[1] is 
CompletionPopup or CompletionPopupAuto, would be broken behaviour too, and 
unexpected by the user; e.g. in Dolphin, typing a path, there's popup list with 
possible completions, the arrow keys navigate through that list.
  
  KRunner is a different beast, it can search files, execute commands, invoke 
kcm modules, manage open windows... etc.
  
  Besides, we are talking about a corner case, 9 out of 10 times you'll only 
get 1 match.
  
  [1] 
https://api.kde.org/frameworks/kcompletion/html/classKCompletion.html#a927c284d89e41d976412201b68ca67e9

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  In D24951#554226 , @ahmadsamir 
wrote:
  
  > In D24951#554138 , @ngraham 
wrote:
  >
  > > Why wouldn't you want the first result chosen when there's more than one 
result? If you enter a search term and get (say) 2 results and the one you want 
to select is the first one, this patch would regress that, no?
  >
  >
  > If there is more than one result, there is no way for the code to know 
which result the user actually wants; please see the test plan for an example 
of when this backfires.
  
  
  Sorry but fixing the issue in the way you're proposing is a UI regression; 
it's just privileging one use case over another (the case where the first item 
is *not* the one you want to launch, vs the case where it is), and does not fix 
the underlying problem.
  
  That underlying problem is that there is no way to use the keyboard to move 
the selection highlight between the results in the list the way, for example, 
KRunner does it: the left and right arrow keys move the insertion point in the 
search field, and the up and down keys navigate through the list of search 
results. That's the way it should be here IMO.
  
  Right now the up and down arrow keys navigate through historical entries in 
the combobox which doesn't really make sense for the use case that this dialog 
supports. KRunner also has a search history but it isn't accessed in this way. 
I think we should just match KRunner's behavior here.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure, ngraham
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[David Faure]

dfaure added a comment.


  +1 from me, the reasoning makes sense.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir added a comment.


  In D24951#554138 , @ngraham wrote:
  
  > Why wouldn't you want the first result chosen when there's more than one 
result? If you enter a search term and get (say) 2 results and the one you want 
to select is the first one, this patch would regress that, no?
  
  
  If there is more than one result, there is no way for the code to know which 
result the user actually wants; please see the test plan for an example of when 
this backfires.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Nathaniel Graham]

ngraham added a comment.


  Why wouldn't you want the first result chosen when there's more than one 
result? If you enter a search term and get (say) 2 results and the one you want 
to select is the first one, this patch would regress that, no?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D24951

To: ahmadsamir, dfaure
Cc: ngraham, kde-frameworks-devel, LeGast00n, GB_2, michaelh, bruns

D24951: [KOpenWithDialog] Automatically select the result if the model filter has only one match

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added a reviewer: dfaure.
Herald added a project: Frameworks.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  If the user types some text in the lineEdit and there's more than one
  match in the model view, don't automatically select the first match, so
  that if the user presses Enter/Return, we don't load the first result
  as it could well be not what the user actually wants.
  
  BUG: 400725

TEST PLAN
  - Make sure ark and wireshark are installed on your system
  - Invoke the open-with dialog on any archive file, and type ark
  - You should get two results (or three if Ark is present in two individual 
categories on your system...) WireShark and Ark, with the former being 
automatically selected for you
  - Press Enter, the file is opened with wireshark
  - Apply the diff then try again, the open-with dialog will not select the 
first result, and if you press Enter the dialog will open the file using the 
command you typed

REPOSITORY
  R241 KIO

BRANCH
  l-kopenwithnondeterministic (branched from master)

REVISION DETAIL
  https://phabricator.kde.org/D24951

AFFECTED FILES
  src/widgets/kopenwithdialog.cpp

To: ahmadsamir, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns