rkflx added a comment.

  > I wonder if having a filter feature even makes any sense when saving files, 
and if it does--why only for Kate?
  
  Well, you should ask #Kate <https://phabricator.kde.org/tag/kate/>. I suspect 
this is because there cannot be a static list of file types, because there are 
so much weird text-based file formats out there. Say you work on `.xyz` files, 
and want to save `complicated_name_1.xyz` and to save on typing you first 
filter for `.xyz` or even for `complicated_name.xyz` which you then simply 
modify to your needs.
  
  Also, KIO is a library, it can be used in all sorts of applications you don't 
even know about. Maybe there is some weird use case out there where files need 
to be saved to places with thousands of entries where filtering is useful…
  
  > If necessary I can make the string return to "Filter" for these 
save-with-filter dialogs, but I'm still left wondering what the feature is 
actually for or if Kate is affected by a bug that should be fixed...
  
  You absolutely should return to "Filter" for this case (even though I agree 
that for most dialogs it does not make sense), because this is 
application-defined behaviour which you don't know a thing about. In particular 
Kate handles it in such a way that what you type in there is //not// appended 
as the file extension. Your change and your help text imply it is, but actually 
trying it out you'll see that Kate won't append the extension (rightly so, 
because filtering for `*.txt` and typing `aa.txt` as the filename should not 
result in `aa.txt.txt`).
  
  At least we'll now have `setFilterLabel` for customization.
  
  ---
  
  > in a new user account
  
  That's a bit pointless in this case, why would you let your reviewers jump 
through such hoops…

INLINE COMMENTS

> kfilewidget.cpp:1340-1341
>                          i18n(autocompletionWhatsThisText);
> +        filterWhatsThisText = i18n("<qt>This is the file type selector. "
> +                         "Use it to choose the type for the file that will 
> be saved.</qt>");
>      } else if (ops->mode() & KFile::Files) {

Personally I hate it when a help text just repeats what's already written in 
the UI. I'd say here you can utilize `…the format the file will be saved in.`, 
which helps out everyone not understanding "type" in the first place, looking 
for help, and then finding something they recognize.

After all, Wikipedia also calls the concept "File format" (keep "type" for the 
label, though).

> kfilewidget.cpp:1344-1345
> +        locationWhatsThisText = "<qt>" + i18n("This is the list of files to 
> open. More than "
>                                        "one file can be specified by listing 
> several "
>                                        "files, separated by spaces.") +
>                          i18n(autocompletionWhatsThisText);

Indentation?

> kfilewidget.h:362
> +     */
> +    void setFilterLabel(const QString &text);
> +

Needs `@since` and possibly `@param`.

This addition to the API might also be worth mentioning in the commit message.

REPOSITORY
  R241 KIO

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

To: ngraham, #frameworks, #vdg, bruns, alexeymin, rkflx, abetts
Cc: davidc, ltoscano, cfeck, rkflx, alexeymin, abetts, bruns, michaelh, ngraham

Reply via email to