Review Request 115611: KoFileDialogHelper => KoFileDialog refactor

[Jarosław Staniek]

> On July 1, 2014, 9:04 p.m., Jarosław Staniek wrote:
> > Hello all,
> > It's so late but I just noticed this API could be improved quite a bit and I am offering to make the fix/review on time for 2.9. Perhaps also some implementation too.
> > 
> > Examples:
> > - use KRecentDirs in KoFileDialog::getUsedDir() to use standard locations as in previous versions of calligra (Recent Dirs gorup) instead of a custom File Dialogs config group
> > - what about non-local URLs in the API? Don't we want to use KIO on Linux/KDE Plasma? Currently I see a 'You can only select local files' message in Sheets
> > - filters: why is the translations and extension list not used from the mime database on Linux/KDE Plasma?
> > - terms could follow better those from Qt/KDElibs, e.g. I had no idea what's a dialogName, it's basically KDE's fileClass I think: http://api.kde.org/4.x-api/kdelibs-apidocs/kio/html/namespaceKRecentDirs.html#a1d58a14171d3269ed2c82288b4ca9661
> > 
> > I already pushed a small obvious fix today to master.
> 
> Boudewijn Rempt wrote:
>     Well, I'd like to avoid any dependency on kio -- it's a tier3 kf5 framework with a wild tangle of dependencies: http://agateau.com/2013/kf5-diagrams/tier3-kio.png. As for krecentdirs -- I'm not sure, but one thing I wanted to achieve here is to have different recent dirs for different file dialogs -- the one for selecting icc profiles should be different from the one for selecting documents, for instance. Plus, that's also in kio.

I see. So I propose a minimal fix at least, config group naming, harmonizing the terms. KRecentDirs' classname is exactly that: different recent dir per file dialog.
Having that I would be able to use KoFileDialog in Kexi more.

Support for remote can come later, transparently, using plugins or so (the only visible API change would be a switch to QUrl).


- Jarosław


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/115611/#review61431
-----------------------------------------------------------


On March 23, 2014, 12:21 a.m., Yue Liu wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115611/
> -----------------------------------------------------------
> 
> (Updated March 23, 2014, 12:21 a.m.)
> 
> 
> Review request for Calligra.
> 
> 
> Repository: calligra
> 
> 
> Description
> -------
> 
> When I'm porting Q/KFileDialog thing to KoFileDialogHelper I found there are some design issues so refactored KoFileDialogHelper.
> 
> Now it names as KoFileDialog and moved to kowidgets.
> 
> Some use cases of file dialog check return value of dialog.exec(), so static methods are abandoned and a getDialog() method is provided to do whatever you want to the QFileDialog object, using static methods also results in parameter list too long.
> 
> Some use cases just use name filters instead of mimetype, so filter setter refactored to 3 methods:
> setNameFilter(QString), setNameFilters(QStringList), setMimeTypeFilters(QStringList)
> 
> Qt 5.2 introduced QFileDialog::setMimeTypeFilters(QStringList) but its not available through static methods, another excuse to abandon static methods.
> 
> Added getXXX() methods as convenience for what static methods usually do.
> 
> 
> Diffs
> -----
> 
>   krita/gemini/desktopviewproxy.cpp 489a440 
>   krita/plugins/assistants/RulerAssistant/kis_ruler_assistant_tool.cc dc4faa2 
>   krita/plugins/extensions/dockers/flipbook/flipbookdocker_dock.cpp ad95ef6 
>   krita/plugins/extensions/dockers/lut/lutdocker_dock.cpp bd5a8e3 
>   krita/plugins/extensions/imagesplit/imagesplit.cpp 1928893 
>   krita/plugins/extensions/separate_channels/kis_channel_separator.cc d6d1769 
>   krita/ui/dialogs/kis_dlg_file_layer.cpp ec5aa52 
>   krita/ui/dialogs/kis_dlg_preferences.cc c02577f 
>   krita/ui/kis_image_manager.cc 8aeddd3 
>   krita/ui/kis_node_manager.cpp 25ca964 
>   krita/ui/widgets/KisFlipbookSelector.cpp 9384790 
>   krita/ui/widgets/kis_color_space_selector.cc 73bde3b 
>   libs/main/CMakeLists.txt ac708c8 
>   libs/main/KoDocument.cpp 70c838c 
>   libs/main/KoFileDialogHelper.h 4afafd4 
>   libs/main/KoFileDialogHelper.cpp 2483bc4 
>   libs/main/KoMainWindow.cpp 11a7248 
>   libs/main/KoOpenPane.cpp bf5b219 
>   libs/widgets/CMakeLists.txt 01f015b 
>   libs/widgets/KoEditColorSetDialog.cpp bf4eabe 
>   libs/widgets/KoFileDialog.h PRE-CREATION 
>   libs/widgets/KoFileDialog.cpp PRE-CREATION 
>   sheets/dialogs/CSVDialog.cpp 4e9d7ed 
> 
> Diff: https://git.reviewboard.kde.org/r/115611/diff/
> 
> 
> Testing
> -------
> 
> Works on OSX.
> 
> But I remember the reason static methods are used is related to some kfiledialog-related bugs on Windows, please test whether KFileDialog is still used when setting UseNativeDialog=true in kdeglobals on Windows.
> 
> 
> Thanks,
> 
> Yue Liu
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/calligra-devel/attachments/20140702/09b664f6/attachment.htm>