Re: Review Request 128283: Add checksums tab to the properties dialog

[Thomas Pfeiffer]

> On July 9, 2016, 1:05 p.m., Ragnar Thomsen wrote:
> > Another post-commit suggestion:
> >   - The "Share" label is not obvious to me. This was also pointed out by 
> > another user on the [blog post](http://www.aelog.org/checksums-made-easy). 
> > Something like "Calculated checksums" would make more sense.
> > 
> > Also +1 for adding "Copy" buttons right of the checksums. And apologies for 
> > not suggesting this before it was committed.
> 
> Elvis Angelaccio wrote:
> What about the "default state" of the dialog? In that case no checksum 
> has been computed yet, so "Calculated checksums" would be misleading maybe?
> 
> But I do agree that "Share" is not perfect.

Maybe "Calculated Checksums" is still okay. Even though originally the 
checksums are not calculated yet, the group is still about those.
Or maybe just not use a group header at all if we can't find one which is not 
misleading in any way?


- Thomas


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


On July 8, 2016, 1:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On July 9, 2016, 1:05 p.m., Ragnar Thomsen wrote:
> > Another post-commit suggestion:
> >   - The "Share" label is not obvious to me. This was also pointed out by 
> > another user on the [blog post](http://www.aelog.org/checksums-made-easy). 
> > Something like "Calculated checksums" would make more sense.
> > 
> > Also +1 for adding "Copy" buttons right of the checksums. And apologies for 
> > not suggesting this before it was committed.

What about the "default state" of the dialog? In that case no checksum has been 
computed yet, so "Calculated checksums" would be misleading maybe?

But I do agree that "Share" is not perfect.


- Elvis


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


On July 8, 2016, 1:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On July 9, 2016, 12:38 p.m., Friedrich W. H. Kossebau wrote:
> > Post-commit suggestions for improvements:
> > The two most usual workflows (checking a checksum, sharing a checksum) here 
> > both require to make use of a (hidden) context menu and searching the one 
> > typical action in that menu.
> > 
> > Please consider:
> > * adding a "Paste" button next to the "expected checksum" field, with 
> > proper tooltip.
> > * adding a "Copy" button next to the calculated checksum label, with proper 
> > tooltip.
> > 
> > That should both simplify usage and also make things more discoverable.

Yes, this is definitely something we need. I didn't add it here because I was 
not sure about the implementation (and this patch was already big enough :P)


- Elvis


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


On July 8, 2016, 1:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Ragnar Thomsen]

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



Another post-commit suggestion:
  - The "Share" label is not obvious to me. This was also pointed out by 
another user on the [blog post](http://www.aelog.org/checksums-made-easy). 
Something like "Calculated checksums" would make more sense.

Also +1 for adding "Copy" buttons right of the checksums. And apologies for not 
suggesting this before it was committed.

- Ragnar Thomsen


On July 8, 2016, 3:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 3:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Friedrich W. H. Kossebau]

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



Post-commit suggestions for improvements:
The two most usual workflows (checking a checksum, sharing a checksum) here 
both require to make use of a (hidden) context menu and searching the one 
typical action in that menu.

Please consider:
* adding a "Paste" button next to the "expected checksum" field, with proper 
tooltip.
* adding a "Copy" button next to the calculated checksum label, with proper 
tooltip.

That should both simplify usage and also make things more discoverable.

- Friedrich W. H. Kossebau


On July 8, 2016, 1:51 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 8, 2016, 1:51 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated July 8, 2016, 1:51 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Submitted with commit c5ab1b80957a993c904f98a86ead880c3cc25421 by Elvis 
Angelaccio to branch master.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated July 8, 2016, 12:38 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Address final issues.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[David Faure]

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


Fix it, then Ship it!





src/widgets/kpropertiesdialog.cpp (line 2720)


Add a comment "not found", otherwise it seems like this is really about Md4.



src/widgets/kpropertiesdialog.cpp (line 2829)


why clear the label if it's going to be hidden anyway?



src/widgets/kpropertiesdialog.cpp (line 2887)


This futureWatcher will be unused if the checksum is in cache.

You could reorder to avoid that: 1) check cache, 2) if present setText and 
return, 3) create future watcher.
Same for the other futurewatcher obviously.


- David Faure


On July 3, 2016, 3:27 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated July 3, 2016, 3:27 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated July 3, 2016, 3:27 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Address David's issues. We use QFileSystemWatcher to invalidate the cache 
whenever the file changes. We don't start anymore a thread if the checksum is 
already in cache.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[David Faure]

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




src/widgets/checksumswidget.ui (line 26)


there two spaces before "are"



src/widgets/checksumswidget.ui (line 48)


two spaces after MD5



src/widgets/checksumswidget.ui (line 55)


same



src/widgets/kpropertiesdialog.cpp (line 2653)


Why the indirection via signal/slots? You could rename slotDefaultState() 
to setDefaultState() and just call it directly where you currently emit 
defaultState. Signals are meant for the "outside" (users of the class), but 
here they are internal.
Same for the other signals below.

I think direct method calls will improve readability, the control flow will 
be easier to determine.



src/widgets/kpropertiesdialog.cpp (line 2661)


This would be done in setDefaultState(), for instance.

If one day an if() has to be added or something, it will be easier ;)



src/widgets/kpropertiesdialog.cpp (line 2772)


This should be an if(), not a tri-state operator returning void.



src/widgets/kpropertiesdialog.cpp (line 2777)


the 3rd argument should be "this"



src/widgets/kpropertiesdialog.cpp (line 2799)


if ()



src/widgets/kpropertiesdialog.cpp (line 2844)


what if the file changes while this dialog is up? (e.g. during a download).

Maybe a change of size or mtime should invalidate the cache?



src/widgets/kpropertiesdialog.cpp (line 2851)


add this as 3rg arg (to avoid crashes if the dialog is closed before this 
finishes)



src/widgets/kpropertiesdialog.cpp (line 2863)


This method is called from the secondary thread, so the private class needs 
a mutex to protect d->m_md5, d->m_sha1 and d->m_sha256

Alternatively, check the cache outside of the method called by 
QtConcurrent. After all it's wasteful to create a QtConcurrent task just to 
realize we have no computation to do.



src/widgets/kpropertiesdialog.cpp (line 2896)


I would make this a file-static method, to make sure you're not accessing 
any member variables from the class from a secondary thread (undefined 
behaviour, without a mutex).

If you remove the cache check and pass the filename as argument, then you 
don't need "this" anymore, it can be static and race-free.



src/widgets/kpropertiesdialog.cpp (line 2899)


double lookup, use a local variable


- David Faure


On June 30, 2016, 8:57 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 30, 2016, 8:57 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the 

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 30, 2016, 8:57 a.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Fix parent-less QFutureWatchers


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Anthony Fieroni]

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




src/widgets/kpropertiesdialog.cpp (line 2776)


I suppose to be *new QFutureWatcher(this)*
Destoying the dialog before future finishes can cause a crash, it think. 
Same below.


- Anthony Fieroni


On Юни 29, 2016, 9:06 след обяд, Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated Юни 29, 2016, 9:06 след обяд)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 29, 2016, 6:06 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

* Don't cache colors anymore.
* Enable exact matching in QRegularExpression patterns.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Friedrich W. H. Kossebau]

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



Nice feature addition :)
Is there a plan for plugins to this plugin, so one could extend it with other 
checksum calculations? Not that serious here though ;)

Here some more quick more serious comments on the code:


src/widgets/kpropertiesdialog.cpp (line 2628)


I propose to not cache the colors here, but instead fetch them when needed. 
There is no real performance gain to cache them.
And the cache might only result in conflicts in case somebody changes the 
system colors while the cache is active.



src/widgets/kpropertiesdialog.cpp (line 2807)


Not sure, but is QRegularExpression doing exact matches by default? IIRC 
other than QRegExp it does not, but also returns true on partial matches in the 
given string.
No time to check in details myself right now, so just asking you to give 
this a check, unless you are sure it is not an issue.


- Friedrich W. H. Kossebau


On June 29, 2016, 2:37 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 29, 2016, 2:37 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 29, 2016, 2:37 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Minor i18n improvements.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 28, 2016, 2:40 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Automatically show a checksum once it's been computed.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Thomas Pfeiffer]

> On June 26, 2016, 12:13 p.m., Luigi Toscano wrote:
> > When the verify operation is executed, shouldn't the computed checksum be 
> > (immediately) shown in the corresponding text box?

Makes sense, since it's then already calculated anyway.


- Thomas


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


On June 27, 2016, 3:46 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 27, 2016, 3:46 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On June 27, 2016, 12:16 p.m., Thomas Pfeiffer wrote:
> > Thank you for implementing the suggestion!
> > There are still some issues with it:
> > 1. HIG says "Avoid using color as a primary method of communication". A 
> > red-green blind person would have no tell success apart from failure here. 
> > Please write "Checksums match" or "Checksums do not match" below.
> > 2. A test I just did with a "regular" user showed that she did not know 
> > what to do with the verification box. I'd suggest writing above it "Copy 
> > and paste a checksum provided for example on the website you downloaded the 
> > file from in the field below."
> > 3.  It would be great if you could insert an expander which shows further 
> > instructions if the checksums don't match, e.g. "This may be due to a 
> > faulty download. Try re-downloading the file. If the verification still 
> > fails, contact the source of the file."
> > 4.  What happens if you type the checksum in manually? At witch point does 
> > it start verifying?
> 
> Elvis Angelaccio wrote:
> > What happens if you type the checksum in manually? At witch point does 
> it start verifying?
> 
> It starts as soon as the input is a valid MD5, SHA1 or SHA256 checksum 
> (which means 32, 40 or 64 hexadecimal digits). If it's not valid, the box 
> becomes red and a tooltip explains what's wrong. I'm assuming that most of 
> the times the user is going to copy-paste the whole checksum.
> 
> > It would be great if you could insert an expander which shows further 
> instructions if the checksums don't match, e.g. "This may be due to a faulty 
> download. Try re-downloading the file. If the verification still fails, 
> contact the source of the file."
> 
> An expander should be doable (using KCollapsibleGroupBox), but if we're 
> going to add a label "Checksums do not match", why not just adding another 
> label below? (with the expanded error message). There is more than enough 
> space in the dialog.
> 
> Thomas Pfeiffer wrote:
> It might look a bit "cleaner" with an expander, but you are right, we 
> have the space and it's not like people have to see the whole thing that 
> often, so it's fine.
> I think we might have made checksum checking actually usable for normal 
> people :) 
> (if they know where to find it, that is)

Thanks! What do you think of the Luigi's proposal? (showing the computed 
checksum in place of the corresponding "Calculate" button)


- Elvis


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


On June 27, 2016, 3:46 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 27, 2016, 3:46 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> 

Re: Review Request 128283: Add checksums tab to the properties dialog

[Thomas Pfeiffer]

> On June 27, 2016, 12:16 p.m., Thomas Pfeiffer wrote:
> > Thank you for implementing the suggestion!
> > There are still some issues with it:
> > 1. HIG says "Avoid using color as a primary method of communication". A 
> > red-green blind person would have no tell success apart from failure here. 
> > Please write "Checksums match" or "Checksums do not match" below.
> > 2. A test I just did with a "regular" user showed that she did not know 
> > what to do with the verification box. I'd suggest writing above it "Copy 
> > and paste a checksum provided for example on the website you downloaded the 
> > file from in the field below."
> > 3.  It would be great if you could insert an expander which shows further 
> > instructions if the checksums don't match, e.g. "This may be due to a 
> > faulty download. Try re-downloading the file. If the verification still 
> > fails, contact the source of the file."
> > 4.  What happens if you type the checksum in manually? At witch point does 
> > it start verifying?
> 
> Elvis Angelaccio wrote:
> > What happens if you type the checksum in manually? At witch point does 
> it start verifying?
> 
> It starts as soon as the input is a valid MD5, SHA1 or SHA256 checksum 
> (which means 32, 40 or 64 hexadecimal digits). If it's not valid, the box 
> becomes red and a tooltip explains what's wrong. I'm assuming that most of 
> the times the user is going to copy-paste the whole checksum.
> 
> > It would be great if you could insert an expander which shows further 
> instructions if the checksums don't match, e.g. "This may be due to a faulty 
> download. Try re-downloading the file. If the verification still fails, 
> contact the source of the file."
> 
> An expander should be doable (using KCollapsibleGroupBox), but if we're 
> going to add a label "Checksums do not match", why not just adding another 
> label below? (with the expanded error message). There is more than enough 
> space in the dialog.

It might look a bit "cleaner" with an expander, but you are right, we have the 
space and it's not like people have to see the whole thing that often, so it's 
fine.
I think we might have made checksum checking actually usable for normal people 
:) 
(if they know where to find it, that is)


- Thomas


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


On June 27, 2016, 3:46 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 27, 2016, 3:46 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
> Mismatch
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
> Match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
> Invalid checksum
>   
> 

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 27, 2016, 3:46 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

* Added a label to explain what the user should do
* Added a label with feedback about the match/mismatch


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments (updated)


MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/ad06e136-7ce3-4876-a594-98fbc64f5538__Spectacle.M13222.png
Mismatch
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/99838e45-d9c5-4a14-b26a-9440f0249c4b__Spectacle.V13222.png
Match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/519fa28f-7c7d-4bb4-bd24-622d18d7f2e2__Spectacle.v13222.png
Invalid checksum
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/27/a243c830-2dc0-4cbd-95e9-8f1684bc86a4__Spectacle.J13336.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On June 27, 2016, 12:16 p.m., Thomas Pfeiffer wrote:
> > Thank you for implementing the suggestion!
> > There are still some issues with it:
> > 1. HIG says "Avoid using color as a primary method of communication". A 
> > red-green blind person would have no tell success apart from failure here. 
> > Please write "Checksums match" or "Checksums do not match" below.
> > 2. A test I just did with a "regular" user showed that she did not know 
> > what to do with the verification box. I'd suggest writing above it "Copy 
> > and paste a checksum provided for example on the website you downloaded the 
> > file from in the field below."
> > 3.  It would be great if you could insert an expander which shows further 
> > instructions if the checksums don't match, e.g. "This may be due to a 
> > faulty download. Try re-downloading the file. If the verification still 
> > fails, contact the source of the file."
> > 4.  What happens if you type the checksum in manually? At witch point does 
> > it start verifying?

> What happens if you type the checksum in manually? At witch point does it 
> start verifying?

It starts as soon as the input is a valid MD5, SHA1 or SHA256 checksum (which 
means 32, 40 or 64 hexadecimal digits). If it's not valid, the box becomes red 
and a tooltip explains what's wrong. I'm assuming that most of the times the 
user is going to copy-paste the whole checksum.

> It would be great if you could insert an expander which shows further 
> instructions if the checksums don't match, e.g. "This may be due to a faulty 
> download. Try re-downloading the file. If the verification still fails, 
> contact the source of the file."

An expander should be doable (using KCollapsibleGroupBox), but if we're going 
to add a label "Checksums do not match", why not just adding another label 
below? (with the expanded error message). There is more than enough space in 
the dialog.


- Elvis


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


On June 26, 2016, 12:03 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 26, 2016, 12:03 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/a776ab78-a61a-47d5-b6e5-9e983310ffb9__Spectacle.M10844.png
> Expected checksum is ok
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/83ce62cf-5264-45f5-a3ff-8aa24dfadc20__Spectacle.B10844.png
> Expected checksum doesn't match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b1363353-c493-4ff6-8320-72d2eeb07c6e__Spectacle.J10844.png
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Thomas Pfeiffer]

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



Thank you for implementing the suggestion!
There are still some issues with it:
1. HIG says "Avoid using color as a primary method of communication". A 
red-green blind person would have no tell success apart from failure here. 
Please write "Checksums match" or "Checksums do not match" below.
2. A test I just did with a "regular" user showed that she did not know what to 
do with the verification box. I'd suggest writing above it "Copy and paste a 
checksum provided for example on the website you downloaded the file from in 
the field below."
3.  It would be great if you could insert an expander which shows further 
instructions if the checksums don't match, e.g. "This may be due to a faulty 
download. Try re-downloading the file. If the verification still fails, contact 
the source of the file."
4.  What happens if you type the checksum in manually? At witch point does it 
start verifying?

- Thomas Pfeiffer


On June 26, 2016, 12:03 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 26, 2016, 12:03 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/a776ab78-a61a-47d5-b6e5-9e983310ffb9__Spectacle.M10844.png
> Expected checksum is ok
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/83ce62cf-5264-45f5-a3ff-8aa24dfadc20__Spectacle.B10844.png
> Expected checksum doesn't match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b1363353-c493-4ff6-8320-72d2eeb07c6e__Spectacle.J10844.png
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Luigi Toscano]

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



When the verify operation is executed, shouldn't the computed checksum be 
(immediately) shown in the corresponding text box?

- Luigi Toscano


On Giu. 26, 2016, 2:03 p.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated Giu. 26, 2016, 2:03 p.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f906577 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa 
>   src/widgets/kpropertiesdialog_p.h c01554e 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> Default dialog
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/a776ab78-a61a-47d5-b6e5-9e983310ffb9__Spectacle.M10844.png
> Expected checksum is ok
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/83ce62cf-5264-45f5-a3ff-8aa24dfadc20__Spectacle.B10844.png
> Expected checksum doesn't match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b1363353-c493-4ff6-8320-72d2eeb07c6e__Spectacle.J10844.png
> MD5 ready to be shared
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 26, 2016, 12:03 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Drop the `QByteArray` buffer and `readAll()`. `QCryptographicHash` accepts a 
`QIODevice` to read data from, so there is no reason to potentially fill the 
ram if the file is huge.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments


Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/a776ab78-a61a-47d5-b6e5-9e983310ffb9__Spectacle.M10844.png
Expected checksum is ok
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/83ce62cf-5264-45f5-a3ff-8aa24dfadc20__Spectacle.B10844.png
Expected checksum doesn't match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b1363353-c493-4ff6-8320-72d2eeb07c6e__Spectacle.J10844.png
MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

2016-06-26 13:04 GMT+02:00 Jaroslaw Staniek :
> readAll() is problematic, why to potentially fill all the ram completely,
> thus removing cache?
> Such approaches can degrade overall plasma's performance.
>

I see that QCryptographicHash accepts a QIODevice to read data from,
I'll try to use this approach.

> Posted from a phone.
>
> On Sunday, 26 June 2016, Elvis Angelaccio 
> wrote:
>> This is an automatically generated e-mail. To reply, visit:
>> https://git.reviewboard.kde.org/r/128283/
>> Review request for KDE Frameworks, KDE Usability and David Faure.
>> By Elvis Angelaccio.
>>
>> Updated June 26, 2016, 10:34 a.m.
>>
>> Changes
>>
>> Implementation of the new layout. Checksums are cached to avoid duplicated
>> computations.
>>
>> Repository: kio
>>
>> Description
>>
>> This patch adds a Checksums tab in the properties dialog, where the user
>> can retrieve and verify the most popular checksum algorithms (md5, sha1 and
>> sha256).
>>
>> To simplify the implementation, the checksums are computed as soon as the
>> user opens the dialog. This can take a while if the file is huge (in
>> particular with sha256), but the computation happens in another thread and
>> in practice this should not be a performance problem.
>>
>> The tab is available only for readable local files (no simlinks) and only
>> when there is a single selection.
>>
>> Please note that some of the labels in the screenshots are clipped due to
>> a bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
>>
>> Testing
>>
>> Check whether the computed values match the values from md5sum, sha1sum,
>> sha256sum.
>> Check whether the line edits get a green background if the computed and
>> expected values match.
>> Check whether the line edits get a red background if the computed and
>> expected values differ.
>>
>> Diffs (updated)
>>
>> src/widgets/CMakeLists.txt (f906577)
>> src/widgets/checksumswidget.ui (PRE-CREATION)
>> src/widgets/kpropertiesdialog.cpp (d0a2faa)
>> src/widgets/kpropertiesdialog_p.h (c01554e)
>>
>> View Diff
>>
>> File Attachments (updated)
>>
>> Default dialog
>> Expected checksum is ok
>> Expected checksum doesn't match
>> MD5 ready to be shared
>
> --
> regards, Jaroslaw Staniek
>
> KDE:
> : A world-wide network of software engineers, artists, writers, translators
> : and facilitators committed to Free Software development - http://kde.org
> Calligra Suite:
> : A graphic art and office suite - http://calligra.org
> Kexi:
> : A visual database apps builder - http://calligra.org/kexi
> Qt Certified Specialist:
> : http://www.linkedin.com/in/jstaniek
>
> ___
> Kde-frameworks-devel mailing list
> Kde-frameworks-devel@kde.org
> https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Jaroslaw Staniek]

readAll() is problematic, why to potentially fill all the ram completely,
thus removing cache?
Such approaches can degrade overall plasma's performance.

Posted from a phone.

On Sunday, 26 June 2016, Elvis Angelaccio 
wrote:
> This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128283/
> Review request for KDE Frameworks, KDE Usability and David Faure.
> By Elvis Angelaccio.
>
> Updated June 26, 2016, 10:34 a.m.
>
> Changes
>
> Implementation of the new layout. Checksums are cached to avoid
duplicated computations.
>
> Repository: kio
>
> Description
>
> This patch adds a Checksums tab in the properties dialog, where the user
can retrieve and verify the most popular checksum algorithms (md5, sha1 and
sha256).
>
> To simplify the implementation, the checksums are computed as soon as the
user opens the dialog. This can take a while if the file is huge (in
particular with sha256), but the computation happens in another thread and
in practice this should not be a performance problem.
>
> The tab is available only for readable local files (no simlinks) and only
when there is a single selection.
>
> Please note that some of the labels in the screenshots are clipped due to
a bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
>
> Testing
>
> Check whether the computed values match the values from md5sum, sha1sum,
sha256sum.
> Check whether the line edits get a green background if the computed and
expected values match.
> Check whether the line edits get a red background if the computed and
expected values differ.
>
> Diffs (updated)
>
> src/widgets/CMakeLists.txt (f906577)
> src/widgets/checksumswidget.ui (PRE-CREATION)
> src/widgets/kpropertiesdialog.cpp (d0a2faa)
> src/widgets/kpropertiesdialog_p.h (c01554e)
>
> View Diff
>
> File Attachments (updated)
>
> Default dialog
> Expected checksum is ok
> Expected checksum doesn't match
> MD5 ready to be shared

-- 
regards, Jaroslaw Staniek

KDE:
: A world-wide network of software engineers, artists, writers, translators
: and facilitators committed to Free Software development - http://kde.org
Calligra Suite:
: A graphic art and office suite - http://calligra.org
Kexi:
: A visual database apps builder - http://calligra.org/kexi
Qt Certified Specialist:
: http://www.linkedin.com/in/jstaniek
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 26, 2016, 10:34 a.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Implementation of the new layout. Checksums are cached to avoid duplicated 
computations.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs (updated)
-

  src/widgets/CMakeLists.txt f906577 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa 
  src/widgets/kpropertiesdialog_p.h c01554e 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments (updated)


Default dialog
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/a776ab78-a61a-47d5-b6e5-9e983310ffb9__Spectacle.M10844.png
Expected checksum is ok
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/83ce62cf-5264-45f5-a3ff-8aa24dfadc20__Spectacle.B10844.png
Expected checksum doesn't match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b1363353-c493-4ff6-8320-72d2eeb07c6e__Spectacle.J10844.png
MD5 ready to be shared
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/26/b882fad9-85b0-4250-9743-3549339e6718__Spectacle.l10844.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

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

(Updated June 25, 2016, 2:52 p.m.)


Review request for KDE Frameworks, KDE Usability and David Faure.


Changes
---

Preview of the new layout (code not yet implemented).

1. The "Verify" line edit becomes either green or red. Green if the pasted 
string is a valid checksum and matches the checksum that we compute in the 
background (according to the length of the input). Red otherwise. Maybe we 
could add a message panel on top to explain the warning/error?
2. In the "Share" groupbox, if the user clicks a "Calculate" button, the button 
is replaced by a "Calculating" label which later will show the resulting 
checksum.


Repository: kio


Description
---

This patch adds a Checksums tab in the properties dialog, where the user can 
retrieve and verify the most popular checksum algorithms (md5, sha1 and 
sha256). 

To simplify the implementation, the checksums are computed as soon as the user 
opens the dialog. This can take a while if the file is huge (in particular with 
sha256), but the computation happens in another thread and in practice this 
should not be a performance problem.

The tab is available only for readable local files (no simlinks) and only when 
there is a single selection.

Please note that some of the labels in the screenshots are clipped due to a bug 
in breeze: https://bugs.kde.org/show_bug.cgi?id=364426


Diffs
-

  src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
  src/widgets/checksumswidget.ui PRE-CREATION 
  src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
  src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 

Diff: https://git.reviewboard.kde.org/r/128283/diff/


Testing
---

* Check whether the computed values match the values from md5sum, sha1sum, 
sha256sum.
* Check whether the line edits get a green background if the computed and 
expected values match.
* Check whether the line edits get a red background if the computed and 
expected values differ.


File Attachments (updated)


sha256-computing
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/327218af-c026-458e-a48d-512787abad42__Spectacle.st2415.png
md5-match
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/4b37d28e-02b9-42e2-96f2-184ffd42b31a__Spectacle.kh2415.png
sha1-differ
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/99fdcf92-f784-4a7d-8aba-c25b8be08c04__Spectacle.Xz2415.png
Preview of new layout
  
https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/ca6efcf4-2b6c-4189-ab05-4e12277f53e2__Spectacle.ZK4766.png


Thanks,

Elvis Angelaccio

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On June 25, 2016, 12:52 p.m., Thomas Pfeiffer wrote:
> > First of all: I love the feature as such, as it makes the whole checksum 
> > comparison story actually usable for average users (although they'd still 
> > have to be told how to do it because they'd probably not get the idea that 
> > they'd find it in the properties dialog)!
> > 
> > I agree with Christoph and David that it shouldn't just calculate all three 
> > checksums as soon as one clicks on the tab.
> > 
> > Let's talk through typical user stories:
> > 
> > I assume the most common story is a user downloading a file from a website 
> > which provides a checksum (either on the site or in the form a text file), 
> > and then wanting to check file integrity using that sum.
> > 
> > For that, the user does not need all three sums (even if all three are 
> > provided, there is little use in checking _all_ of them, is there?).
> > From an average user's perspective, the ideal situation would be that 
> > they's simply have to paste the checksum they have into a generic checksum 
> > field, then it would be automatically detect which type of checksum it is, 
> > the corresponding checksum would be calculated and the comparison would be 
> > made. Would that be possible?
> > 
> > We have to be aware that the whole checksum business is pretty advanced and 
> > difficult to understand for most users, which is why I fear that it is far 
> > less often used than it should be. Therefore, the less knowledge about 
> > checksums a user needs in order to complete the task, the better.
> > 
> > 
> > Another story would be a user wanting to share the actual checksum 
> > somewhere. For that, they'd need a way to generate it manually. Even then, 
> > however, how likely is it they'd need all three?
> > For this story I'd provide either three different "Calculate Checksum" 
> > buttons or one drop down button to select which one to calculate, and then 
> > calculate only that.

Thanks for the feedback!

> I agree with Christoph and David that it shouldn't just calculate all three 
> checksums as soon as one clicks on the tab.

Got it, indeed it's not that useful to show all 3 of them at the same time.

> From an average user's perspective, the ideal situation would be that they's 
> simply have to paste the checksum they have into a generic checksum field, 
> then it would be automatically detect which type of checksum it is, the 
> corresponding checksum would be calculated and the comparison would be made. 
> Would that be possible?

I think it should be possible. We could check the length of the pasted checksum 
and guess the type of algorithm from that.

> Another story would be a user wanting to share the actual checksum somewhere. 
> For that, they'd need a way to generate it manually. Even then, however, how 
> likely is it they'd need all three?
For this story I'd provide either three different "Calculate Checksum" buttons 
or one drop down button to select which one to calculate, and then calculate 
only that.

Makes sense as well. Using a dropdown button should be the simplest possible 
solution.


- Elvis


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


On June 25, 2016, 10:39 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 25, 2016, 10:39 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
>   src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, 

Re: Review Request 128283: Add checksums tab to the properties dialog

[Elvis Angelaccio]

> On June 25, 2016, 11:35 a.m., Christoph Feck wrote:
> > I wished it was for the KIO overwrite dialog ;)
> > 
> > Anyway, reading the file three times without the user asking for the 
> > checksums is a recipe for disk cache draining.
> > 
> > I suggest to compute all checksums in parallel, i.e. read the file only 
> > once, and only do it when the user selects the Checksum tab.

I'm not sure where I'm reading the file from disk more than once. There is only 
one `file.readAll()` in the constructor. Maybe when the file doesn't fit in ram 
and reading (later) from `buffer` implies reading from the disk swap?


- Elvis


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


On June 25, 2016, 10:39 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 25, 2016, 10:39 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
>   src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> sha256-computing
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/327218af-c026-458e-a48d-512787abad42__Spectacle.st2415.png
> md5-match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/4b37d28e-02b9-42e2-96f2-184ffd42b31a__Spectacle.kh2415.png
> sha1-differ
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/99fdcf92-f784-4a7d-8aba-c25b8be08c04__Spectacle.Xz2415.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Thomas Pfeiffer]

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



First of all: I love the feature as such, as it makes the whole checksum 
comparison story actually usable for average users (although they'd still have 
to be told how to do it because they'd probably not get the idea that they'd 
find it in the properties dialog)!

I agree with Christoph and David that it shouldn't just calculate all three 
checksums as soon as one clicks on the tab.

Let's talk through typical user stories:

I assume the most common story is a user downloading a file from a website 
which provides a checksum (either on the site or in the form a text file), and 
then wanting to check file integrity using that sum.

For that, the user does not need all three sums (even if all three are 
provided, there is little use in checking _all_ of them, is there?).
>From an average user's perspective, the ideal situation would be that they's 
>simply have to paste the checksum they have into a generic checksum field, 
>then it would be automatically detect which type of checksum it is, the 
>corresponding checksum would be calculated and the comparison would be made. 
>Would that be possible?

We have to be aware that the whole checksum business is pretty advanced and 
difficult to understand for most users, which is why I fear that it is far less 
often used than it should be. Therefore, the less knowledge about checksums a 
user needs in order to complete the task, the better.


Another story would be a user wanting to share the actual checksum somewhere. 
For that, they'd need a way to generate it manually. Even then, however, how 
likely is it they'd need all three?
For this story I'd provide either three different "Calculate Checksum" buttons 
or one drop down button to select which one to calculate, and then calculate 
only that.

- Thomas Pfeiffer


On June 25, 2016, 10:39 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 25, 2016, 10:39 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
>   src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> sha256-computing
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/327218af-c026-458e-a48d-512787abad42__Spectacle.st2415.png
> md5-match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/4b37d28e-02b9-42e2-96f2-184ffd42b31a__Spectacle.kh2415.png
> sha1-differ
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/99fdcf92-f784-4a7d-8aba-c25b8be08c04__Spectacle.Xz2415.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[David Faure]

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



Nice feature, but please only start the calculation when clicking on a 
"Calculate" button, like I did for the "disk space usage" calculation. 99% of 
the users of the dialog won't need the checksums and the corresponding disk 
activity, so better make it explicit.

- David Faure


On June 25, 2016, 10:39 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 25, 2016, 10:39 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
>   src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> sha256-computing
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/327218af-c026-458e-a48d-512787abad42__Spectacle.st2415.png
> md5-match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/4b37d28e-02b9-42e2-96f2-184ffd42b31a__Spectacle.kh2415.png
> sha1-differ
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/99fdcf92-f784-4a7d-8aba-c25b8be08c04__Spectacle.Xz2415.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 128283: Add checksums tab to the properties dialog

[Christoph Feck]

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



I wished it was for the KIO overwrite dialog ;)

Anyway, reading the file three times without the user asking for the checksums 
is a recipe for disk cache draining.

I suggest to compute all checksums in parallel, i.e. read the file only once, 
and only do it when the user selects the Checksum tab.

- Christoph Feck


On June 25, 2016, 10:39 a.m., Elvis Angelaccio wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128283/
> ---
> 
> (Updated June 25, 2016, 10:39 a.m.)
> 
> 
> Review request for KDE Frameworks, KDE Usability and David Faure.
> 
> 
> Repository: kio
> 
> 
> Description
> ---
> 
> This patch adds a Checksums tab in the properties dialog, where the user can 
> retrieve and verify the most popular checksum algorithms (md5, sha1 and 
> sha256). 
> 
> To simplify the implementation, the checksums are computed as soon as the 
> user opens the dialog. This can take a while if the file is huge (in 
> particular with sha256), but the computation happens in another thread and in 
> practice this should not be a performance problem.
> 
> The tab is available only for readable local files (no simlinks) and only 
> when there is a single selection.
> 
> Please note that some of the labels in the screenshots are clipped due to a 
> bug in breeze: https://bugs.kde.org/show_bug.cgi?id=364426
> 
> 
> Diffs
> -
> 
>   src/widgets/CMakeLists.txt f9065777a0d2f1d126dc482d66e1bbb1ba127895 
>   src/widgets/checksumswidget.ui PRE-CREATION 
>   src/widgets/kpropertiesdialog.cpp d0a2faa5114e391680925e10646ce7c6f2ea29da 
>   src/widgets/kpropertiesdialog_p.h c01554e694ff3c905e768048ce94800739760fb7 
> 
> Diff: https://git.reviewboard.kde.org/r/128283/diff/
> 
> 
> Testing
> ---
> 
> * Check whether the computed values match the values from md5sum, sha1sum, 
> sha256sum.
> * Check whether the line edits get a green background if the computed and 
> expected values match.
> * Check whether the line edits get a red background if the computed and 
> expected values differ.
> 
> 
> File Attachments
> 
> 
> sha256-computing
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/327218af-c026-458e-a48d-512787abad42__Spectacle.st2415.png
> md5-match
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/4b37d28e-02b9-42e2-96f2-184ffd42b31a__Spectacle.kh2415.png
> sha1-differ
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2016/06/25/99fdcf92-f784-4a7d-8aba-c25b8be08c04__Spectacle.Xz2415.png
> 
> 
> Thanks,
> 
> Elvis Angelaccio
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel