Review Request 111091: Add "Always Ask" option for filetransfer destination directory

Daniele E. Domenichelli daniele.domenichelli at gmail.com
Thu Jun 20 08:33:45 UTC 2013



> On June 19, 2013, 12:16 a.m., Daniele E. Domenichelli wrote:
> > I thought a bit about this patch, and I'm always more convinced that this shouldn't be in the file transfer handler, but in the approver (and that's the reason why I never added it).
> > Imho the approver should have a button to accept and one to change directory, the "Always Ask" is just one extra configuration option that will clutter the ui and will prompt the user every time, even when it is not needed because he just wants to download the file in his download directory
> 
> Dan Vrátil wrote:
>     Also I don't see how you would want to pass the folder name from approver to ft-handler.
>     
>     I see the idea of "Always Ask" as an analogy to behavior of web browsers. In browsers, you usually only have these two options as well. You either have to confirm every single file, or every downloaded is saved to a user defined directory.
>     
>     The usecase of the "Always Ask" option is that users want to keep their files in order. They don't want to have huuuuge ~/Downloads directory, but they rather want to filter incoming files into folders right away. For such people the "when it is not needed" situation does occur only very rarely.
>     
>     Now thinking about the analogy with browsers, when using the "Always Ask" option, browsers usually remember the last opened folder. This would fix your case when someone is sending you multiple files: you only need to set the folder for the first time; next time the same folder will already selected, so you just confirm the dialog. Would that work for you?
>     
>

The approver could just overwrite the downloadDirectory entry, and when the ft-handler reads it, it will read the new one.
Why should we do like browsers, when we can do better? Anyway if you remember the last folder, until we get a notification system that is able to handle this, it is ok for me. But instead of checking the empty string, I'd rather add a bool to the ctor of the HandleIncomingFileTransferChannelJob class and always read both parameters.

By the way, I just noticed that in the header, the string passed to the class is "outputFileName" and in the implementation is "downloadDirectory". Since you are touching this, can you change it to downloadDirectory everywhere?


- Daniele E.


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111091/#review34635
-----------------------------------------------------------


On June 18, 2013, 1:37 p.m., Dan Vrátil wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111091/
> -----------------------------------------------------------
> 
> (Updated June 18, 2013, 1:37 p.m.)
> 
> 
> Review request for Telepathy.
> 
> 
> Description
> -------
> 
> Implements support for "Always Ask" for filetransfer destination directory option.
> 
> The hack with __k__onDownloadDirectoryCancelled() is necessary because KDirSelectDialog::result() always returns 0 and buttonClicked() signal is emitted even when "New Folder" button is clicked and handling that would make the code more complicated.
> 
> 
> This addresses bug 295937.
>     http://bugs.kde.org/show_bug.cgi?id=295937
> 
> 
> Diffs
> -----
> 
>   src/CMakeLists.txt ebd892c 
>   src/filetransfer-handler.cpp 74c701e 
>   src/handle-incoming-file-transfer-channel-job.h d5e53c4 
>   src/handle-incoming-file-transfer-channel-job.cpp 7a15237 
> 
> Diff: http://git.reviewboard.kde.org/r/111091/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Dan Vrátil
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130620/2a37453a/attachment-0001.html>


More information about the KDE-Telepathy mailing list