[Kde-pim] Re: Review Request: Output error message when saving an attachment to a disk, which becomes full - Bug 127981

[Ingo Klöcker]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/6104/#review9209
-----------------------------------------------------------


In general, the patch looks good. Thanks for fixing this bug.

I'm not sure it makes much sense to differentiate the two cases bytesWritten == -1 and -1 < bytesWritten < data.size() duplicating quite some code. I suggest combining both.

Your patch introduces a new string. Therefore you cannot commit now since we are in string freeze for KDE 4.6. Combining both cases and using the old error message would solve this.

Please fix the indentation of the code inside the if-clause while you are touching the surrounding code. In kdepim we use 2 spaces.


/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10081>

    Please move this line further down to where it's first needed.



/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10082>

    I'd prefer you to split this in two lines. In fact, you can combine the assignment with the declaration of bytesWritten and then make bytesWritten const.



/trunk/KDE/kdepim/messageviewer/util.cpp
<http://svn.reviewboard.kde.org/r/6104/#comment10083>

    There should probably be a <br> after </filename>


- Ingo


On 2010-12-12 16:43:22, George  Metaxas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/6104/
> -----------------------------------------------------------
> 
> (Updated 2010-12-12 16:43:22)
> 
> 
> Review request for KDE PIM.
> 
> 
> Summary
> -------
> 
> As discussed in Bug 127981, when one attempts to save an attachment to a disk which has some space available, but not enough to fit the attachment, KMail will not output an error message informing the user about this. Instead, the attachment will be partially saved and the user will be oblivious to what really happened. This patch will show an error message to the user and will also remove the partially written file. KMail does indeed correctly inform the user if the disk has no space left, but it leaves behind a zero-sized file. This patch also removes this unnecessary file.
> 
> 
> This addresses bug 127981.
>     https://bugs.kde.org/show_bug.cgi?id=127981
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdepim/messageviewer/util.cpp 1205704 
> 
> Diff: http://svn.reviewboard.kde.org/r/6104/diff
> 
> 
> Testing
> -------
> 
> Tested saving an attachment with:
> - No space left
> - Some space left, but not enough to fit the attachment
> - Enough space to fit the attachment
> 
> 
> Thanks,
> 
> George
> 
>

_______________________________________________
KDE PIM mailing list kde-pim at kde.org
https://mail.kde.org/mailman/listinfo/kde-pim
KDE PIM home page at http://pim.kde.org/