Review Request 115720: Merged DolphinMainWindow::openDirectories() and DolphinMainWindow::openFiles() -> DolphinMainWindow::openUrls()

Frank Reininghaus frank78ac at googlemail.com
Mon Feb 17 17:20:52 GMT 2014


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


Thanks for the patch. The suggested feature might be useful in some situations, but I see a couple of potential problems.

You suggest to drop the --select option, and make the code smart instead and let it guess if the user meant to open or to select the given URLs. In my experience, code that tries to be smart and guesses what the user wants might fail at some point, and then the user has no way to get what they really want.

Moreover, if a user does the following:

dolphin --select /path/to/a.txt /path/to/b.txt /path/to/c.txt

Dolphin will now open the URL /path/to and then select the three files. With your patch, it would open three different views, all showing /path/to, and select a different file in each of them, right? This is probably not what the user wants.

Finally, if the user wants to open a URL and select a large number of files, your version will call determineMimeType() for every file. This might be slow and block the entire application for a long time.

- Frank Reininghaus


On Feb. 13, 2014, 12:10 p.m., Emmanuel Pescosta wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/115720/
> -----------------------------------------------------------
> 
> (Updated Feb. 13, 2014, 12:10 p.m.)
> 
> 
> Review request for Dolphin.
> 
> 
> Repository: kde-baseapps
> 
> 
> Description
> -------
> 
> Merged DolphinMainWindow::openDirectories() and DolphinMainWindow::openFiles() -> DolphinMainWindow::openUrls()
> 
> We use the given url now and extract the directory and item url. If the item url is valid, we mark
> the file as current.
> 
> --select console arg is now obsolete, because you can simply write the full url instead. (e.g. /home/konq/me.png)
> 
> Open path in new tab/window also make use of these changes, files opened from search queries are now
> marked as current in the new tab/window.
> 
> 
> Diffs
> -----
> 
>   dolphin/src/dolphinapplication.cpp 8e83a85 
>   dolphin/src/dolphinmainwindow.h cb97612 
>   dolphin/src/dolphinmainwindow.cpp 8473014 
>   dolphin/src/dolphinviewcontainer.h 31612f1 
>   dolphin/src/dolphinviewcontainer.cpp 768fd5e 
> 
> Diff: https://git.reviewboard.kde.org/r/115720/diff/
> 
> 
> Testing
> -------
> 
> dolphin -> opens home dir
> dolphin "/home/emmanuel/Downloads/Aufgabe.pdf" -> opens Downloads, marks Aufgabe.pdf
> dolphin "tags:/Kde" -> opens tags:/Kde
> dolphin "/home/emmanuel/zipped.zip" -> opens "zip:/home/emmanuel/zipped.zip"
> 
> searching something and open path in new tab, openes folder and marks file.
> 
> 
> Thanks,
> 
> Emmanuel Pescosta
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20140217/532eeb2e/attachment.htm>


More information about the kfm-devel mailing list