Re: Review Request: Show warning when CopyJob fails to list a subdir

2012-09-04 Thread David Faure

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

Ship it!


Ship It!

- David Faure


On Aug. 22, 2012, 8:24 a.m., Dan Vratil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/106052/
 ---
 
 (Updated Aug. 22, 2012, 8:24 a.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Description
 ---
 
 Display a warning when CopyJob fails to enter a subdirectory and thus can't 
 copy it's content.
 
 
 Diffs
 -
 
   kdeui/jobs/kdialogjobuidelegate.cpp fe48f87 
   kio/kio/copyjob.h eb88c7a 
   kio/kio/copyjob.cpp 8dde763 
   kio/kio/job.cpp a7e1baf 
   kio/kio/jobclasses.h de27f40 
 
 Diff: http://git.reviewboard.kde.org/r/106052/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dan Vratil
 




Re: Review Request: Show warning when CopyJob fails to list a subdir

2012-09-04 Thread Commit Hook

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


This review has been submitted with commit 
a1b525d29af172e2c983643ee525bf69f541f34c by Dan Vrátil to branch KDE/4.9.

- Commit Hook


On Aug. 22, 2012, 8:24 a.m., Dan Vratil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/106052/
 ---
 
 (Updated Aug. 22, 2012, 8:24 a.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Description
 ---
 
 Display a warning when CopyJob fails to enter a subdirectory and thus can't 
 copy it's content.
 
 
 Diffs
 -
 
   kdeui/jobs/kdialogjobuidelegate.cpp fe48f87 
   kio/kio/copyjob.h eb88c7a 
   kio/kio/copyjob.cpp 8dde763 
   kio/kio/job.cpp a7e1baf 
   kio/kio/jobclasses.h de27f40 
 
 Diff: http://git.reviewboard.kde.org/r/106052/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dan Vratil
 




Re: Review Request: Show warning when CopyJob fails to list a subdir

2012-08-16 Thread David Faure

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


 I think the right thing to do is to fix bug 206500, and then apply this patch, 
rather than come up with yet another feedback mechanism.
Let's make warning() actually work, and let's make sure it's not emitted 
erroneously.

I'll look into fixing bug 206500 as soon as possible (let's say beginning of 
next week).


kdeui/jobs/kdialogjobuidelegate.cpp
http://git.reviewboard.kde.org/r/106052/#comment13720

No space after the method name (this usually happens when copy/pasting from 
the Qt documentation :-)

In fact, no space within the parenthesis either, but you couldn't guess 
that: existing code doesn't follow the kdelibs coding style. When writing new 
code, no spaces within (...).



kio/kio/job.cpp
http://git.reviewboard.kde.org/r/106052/#comment13721

Left - Let. How did this come in? It wasn't in my initial patch :)


- David Faure


On Aug. 16, 2012, 12:18 p.m., Dan Vratil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/106052/
 ---
 
 (Updated Aug. 16, 2012, 12:18 p.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Description
 ---
 
 Display a warning when CopyJob fails to enter a subdirectory and thus can't 
 copy it's content.
 
 
 Diffs
 -
 
   kdeui/jobs/kdialogjobuidelegate.cpp fe48f87 
   kio/kio/copyjob.h eb88c7a 
   kio/kio/copyjob.cpp 8dde763 
   kio/kio/job.cpp a7e1baf 
   kio/kio/jobclasses.h de27f40 
 
 Diff: http://git.reviewboard.kde.org/r/106052/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dan Vratil
 




Re: Review Request: Show warning when CopyJob fails to list a subdir

2012-08-16 Thread Ambroz Bizjak


 On Aug. 16, 2012, 1:10 p.m., Frank Reininghaus wrote:
  kdeui/jobs/kdialogjobuidelegate.cpp, line 92
  http://git.reviewboard.kde.org/r/106052/diff/1/?file=78076#file78076line92
 
  I'm afraid the users suffering from 
  https://bugs.kde.org/show_bug.cgi?id=206500 will kill us if they get a 
  message box for every single file. Right now, they have the option to wait 
  until the operation is completed, then close the first (and only) message 
  box and be happy.
 
 Ambroz Bizjak wrote:
 How about aggregating all the errors/warnings in a single dialog box as a 
 list? E.g. the list would have fields like message and file, and would 
 allow the user to easily see all that went wrong and what files were 
 involved. He could then select multiple entries and perform the same action 
 on them, if applicable. E.g. if the message was destination already exists, 
 replace or skip? he could select some of the entries and perform ignore, 
 but perform replace on others. The copying could continue in the 
 background, and the replacements would be performed only once the user 
 confirms then.
 
 Dan Vratil wrote:
 But you can get multiple kinds of warnings during the operation. How 
 would you separate them? You would either have to have a separate dialog for 
 each kind of error or a very complex UI dialog. In general I like the idea of 
 a box These files failed to copy (similar to the Delete Files dialog), but 
 I think that simple 'OK' would be enough. Ideas?

Yes, that's a good idea. When a file fails to copy, pop up the These files 
failed to copy dialog, but when another file fails, just add it to the list in 
the existing dialog. User can click OK which closes the dialog, and if more 
files fail, another one pops up. Small problem though: what happens if the user 
clicks OK, without noticing that another file was added to the list just before 
he closed the dialog? Maybe a safeguard should be added that no less than N 
seconds may pass from the time the last file was added to when the user clicks 
OK.


- Ambroz


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


On Aug. 16, 2012, 12:18 p.m., Dan Vratil wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 http://git.reviewboard.kde.org/r/106052/
 ---
 
 (Updated Aug. 16, 2012, 12:18 p.m.)
 
 
 Review request for kdelibs and David Faure.
 
 
 Description
 ---
 
 Display a warning when CopyJob fails to enter a subdirectory and thus can't 
 copy it's content.
 
 
 Diffs
 -
 
   kdeui/jobs/kdialogjobuidelegate.cpp fe48f87 
   kio/kio/copyjob.h eb88c7a 
   kio/kio/copyjob.cpp 8dde763 
   kio/kio/job.cpp a7e1baf 
   kio/kio/jobclasses.h de27f40 
 
 Diff: http://git.reviewboard.kde.org/r/106052/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Dan Vratil