rkflx accepted this revision.
rkflx added a comment.
This revision is now accepted and ready to land.

  In D8056#205595 <https://phabricator.kde.org/D8056#205595>, @simgunz wrote:
  > Fixed the problems reported by @rkflx  hopefully without introducing new 
ones. I did some testing and now everything seems ok, but please check again.
  Thanks Simone, works great now.
  Successfully tested opening a file with the following methods:
  - mouse selection only
  - type a few characters, select with the mouse
  - type a few characers, hit [Enter]
  - double click on entry
  - history dropdown
  - enable completion dropdown, open with custom bin in path
  - open button
  - paste custom bin not in path
  - autocomplete custom path
  - [↓] to move focus (this breaks with the completion enabled, though)
  That's basically all the dialog should be able to do that I know of, 
therefore we can land it (once the tooltip is changed).
  For improving the usability, I still want to open a task with some ideas to 
discuss them further (really soon™, the first step is done by listing the 
current functionality ↑↑↑). [My promise for Nate's 
https://phabricator.kde.org/D10245 is still open too…]
  In D8056#205608 <https://phabricator.kde.org/D8056#205608>, @simgunz wrote:
  > The tooltip of the line edit now says: `Type some text to filter the 
application tree.\nPress down arrow to navigate the results tree.`
  >  I suggest to change it to: `Type to filter the application tree or type 
the name of a command.\nPress down arrow to navigate the results tree.` just to 
avoid having a hidden feature, which is the possibility to type commands. Also 
because the completion mode default is now set to None, so there will not even 
be the completion that suggests the existence of this feature.
  You are right, the least we can do for now is to improve the tooltip. I don't 
think users know what a "tree" is, so how about this:
    Type to filter the applications below, or specify the name of a command.
    Press down arrow to navigate the results.
  Use/adapt it if you like it, I'll wait with landing.
  > When you click OK the function `checkAccepted` is called. To me it seems 
that it actually uses the service associated to the selected "Kate" entry 
(`kate -b %U`) and does not execute the command `kate`. 
  >  (That function is quite complicated and I haven't really analyzed it 
  Yeah, that's not ideal (but not too bad since the completion is off by 
default). In my testing the service was also preferred over the executable, 
which is fine because you can explicitly specify the latter by typing the full 


> kopenwithdialog.cpp:834-836
> -    if (typedExec.isEmpty()) {
> -        return false;
> -    }

I checked this, but AFAIK it's fine to remove. @dfaure introduced this 10 years 
ago in R446:d6903613f07b 
but I guess we are covered both by the disabled button for empty line edits as 
well as the checks below.

  R241 KIO



To: simgunz, dfaure, #frameworks, #vdg, ngraham, rkflx
Cc: rkflx, romangg, fabianr, abetts, ngraham, alexeymin, #frameworks, michaelh

Reply via email to