Re: Multiselection needs work?

On 13/07/2015 13:33, Julien Nabet wrote:
> 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?
pending review: https://gerrit.libreoffice.org/#/c/17025/
> 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?
pending review: https://gerrit.libreoffice.org/#/c/17064/
> ...
>
> 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.
Except the already done patch for Vista, I'm a bit lost for those. I 
mean, it seems it can only retrieve 1 file but not sure...
> 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
Taking another look, I think the 2 first are completely specific to 
migration part so don't think it's a good idea to change them. However, 
there could be some common functions, at least getFiles which is a mere 
copy/paste between them.

officefilepicker: https://gerrit.libreoffice.org/#/c/17068/ (at least it 
builds ok on my laptop)

Julien