Re: Multiselection needs work?

On 13/07/2015 10:44, Michael Meeks wrote:
> Hi Julien,
>
> On Sun, 2015-07-12 at 01:02 +0200, Julien Nabet wrote:
> > Here's a patch: https://gerrit.libreoffice.org/#/c/16959/
> > I truncated to 1 file in KDE part + Vista part.
>         Heh =) I worry though that the implementation of the KDE & Vista bits
> is doing this over-complicated foo - wrt. storing the directory URL in
> the first item, and then relative file-names in the next items, so by
> truncating it to 1x long we end up with just the directory-URL not the
> first file that was selected =)
>
>         I'd hope we could rip some complexity out of the implementations there.
For kde4, it seems ok, see 
http://opengrok.libreoffice.org/xref/core/vcl/unx/kde4/KDE4FilePicker.cxx#337
but KDE4FilePicker::getSelectedFiles(), some lines bellow, uses 
getFiles! So it'll quickly need some fix, I thought about a copy paste 
of the original version of getFiles. In this case perhaps getFiles 
should use getSelectedFiles() then truncate?

For kde, it seems there are 2 implementations:
- 
http://opengrok.libreoffice.org/xref/core/vcl/unx/kde/fpicker/kdefilepicker.cxx#322
- 
http://opengrok.libreoffice.org/xref/core/vcl/unx/kde/UnxCommandThread.cxx#103
a bit confused here, should I blindly the same logic?

aqua: it seems you were right yo be worried
http://opengrok.libreoffice.org/xref/core/fpicker/source/aqua/SalAquaFilePicker.mm#304
I submitted a patch to review (blindly because building on Mac still 
fails for me, see 
https://bugs.documentfoundation.org/show_bug.cgi?id=90502 ), see 
fpicker/source/aqua/SalAquaFilePicker.mm

Windows world:
CNonExecuteFilePickerState::getFiles 
(http://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/filepickerstate.cxx#226)
CExecuteFilePickerState::getFiles 
(http://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/filepickerstate.cxx#510)
I could apply the same logic.

+ already indicated 
http://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/VistaFilePicker.cxx#255 
and already dealt with this patch
http://cgit.freedesktop.org/libreoffice/core/diff/fpicker/source/win32/filepicker/VistaFilePicker.cxx?id=d11b244bf9b9115f5384d6ff43bdffc7f1289d71

There are also "wrappers like":
http://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/WinFileOpenImpl.cxx#172
http://opengrok.libreoffice.org/xref/core/fpicker/source/win32/filepicker/FilePicker.cxx#365
Perhaps just a comment there should be sufficient.

Others:
http://opengrok.libreoffice.org/xref/core/desktop/source/migration/services/basicmigration.cxx#73
http://opengrok.libreoffice.org/xref/core/desktop/source/migration/services/wordbookmigration.cxx#93
officefilepicker: 
http://opengrok.libreoffice.org/xref/core/fpicker/source/office/OfficeFilePicker.cxx#589
what's this? If it's really used, it should be indeed patched.

Hope I spotted them all.

May we suppose too that one of them is used for macro Basic language 
since it seems public API?
>         I'd also suggest killing the comment about multi-selection in the UNO
> IDL and saying something of the form: "This always, only ever returns
> the first selected file URL" or somesuch (?) =)
Here's the patch waiting for review:
https://gerrit.libreoffice.org/16985

Julien