Add workaround for gtk file chooser filter patterns
ClosedPublic

Authored by jgrulich on Dec 19 2018, 9:26 AM.

Details

Summary

Gtk can use filter patterns, looking like "*.[Pp][Nn][Gg]" to handle all possible lowercase/uppercase
variants. This is unfortunately not supported by KDE/Qt dialogs so we should try to convert it to
just "*.png" instead. There are other possible patterns, but this is probably the most common one.

BUG: 399889

Diff Detail

Repository
R838 Flatpak Support: KDE Portal for XDG Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jgrulich created this revision.Dec 19 2018, 9:26 AM
Restricted Application added a project: Plasma. · View Herald TranscriptDec 19 2018, 9:26 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
jgrulich requested review of this revision.Dec 19 2018, 9:26 AM
jgrulich edited the summary of this revision. (Show Details)Dec 19 2018, 9:33 AM
jgrulich updated this revision to Diff 47829.Dec 19 2018, 9:44 AM

Be more specific about the pattern

jgrulich updated this revision to Diff 47833.Dec 19 2018, 9:57 AM

Allow also patterns like '*.ab[cC]de[fF]'

davidedmundson added inline comments.
src/filechooser.cpp
287

Given this is the plasma portal, it might be a good case to use the KFileWidget directly rather than have a proxy layer in the way which converts filter strings back again.

KFileWidget::setFilter takes an expression directly - and if it doesn't match perfectly we can change KIO to come up with something that does.

jgrulich added inline comments.Dec 19 2018, 1:46 PM
src/filechooser.cpp
287

This can be done of course, but it will not fix this problem. It's reasonable to use it directly and maybe add support for regular expressions in filters to KIO, but this is something what will need more work and require changes on two places. I would like to have now a workaround that can be pushed to stable Plasma release now.

The fixed bug is actually 399889

jgrulich edited the summary of this revision. (Show Details)Dec 19 2018, 3:02 PM
src/filechooser.cpp
287

KFileWidget::setFilter exists and takes regular expressions already.

jgrulich added inline comments.Dec 20 2018, 12:01 PM
src/filechooser.cpp
287

Does it really support regexp in this form? If so, why it doesn't work when used by plasma-integration? I see plasma-integration internally uses KFileWidget and passes filters to it.

davidedmundson added inline comments.Dec 20 2018, 3:32 PM
src/filechooser.cpp
287

It should do:

It goes through:

QRegExp rx(p);
      rx.setPatternSyntax(QRegExp::Wildcard);

Wildcard in the Qt docs sounds very much like the "glob" format that's in the XDG spec.

Running ./kfilewidgettest_gui in kio/bin you can type a filter manually

I did:
*.[Pp][Dd][Ff] and that worked for me.

It doesn't work in plasma-integration because it goes through a method qt2KdeFilter - to turn Qt's weird custom syntax into reg ex patterns.

jgrulich abandoned this revision.Dec 21 2018, 9:09 AM

Is there a new version of this patch somewhere else?

Is there a new version of this patch somewhere else?

Not yet, will be soon.

I have now a new version using KFileWidget and it still suffers from the same problem. It does filter properly when I use regexp, but the "automatic extension" feature will add the file extension unmodified so it results into the same problem. It looks that my workaround will be needed anyway.

but the "automatic extension" feature will add the file extension unmodified

Can you expand on what this means.

jgrulich reclaimed this revision.Dec 22 2018, 5:02 PM

but the "automatic extension" feature will add the file extension unmodified

Can you expand on what this means.

If you try to save a file and you use for example *.[Pp][Nn][Gg] as a filter, the "automaticly select filename extension" checkbox in the dialog will automatically set "filename.[Pp][Nn][Gg]" as the selected file, same happens if you switch beetween filters.

Thanks for working on this. Given we need a kio patch if you want to put this in just the stable branch, go for it.

David, can you accept this revision then? I'll push it to Plasma/5.14 branch only.

davidedmundson accepted this revision.Sun, Dec 23, 11:42 AM
This revision is now accepted and ready to land.Sun, Dec 23, 11:42 AM
This revision was automatically updated to reflect the committed changes.