D8536: Add more hashing algorithms to KPropertiesDialog

2018-08-22 Thread Peter Majchrak
petermajchrak added a comment.


  @ngraham I have in the meantime switched away from KDE, so it would be 
difficult for me to continue the development...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: kde-frameworks-devel, broulik, colomar, anthonyfieroni, bcooksley, 
alexeymin, ngraham, elvisangelaccio, michaelh, bruns


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added a comment.


  Is there a way to separate this class to its own file? Or is it some 
convention in this project to have these huge files?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> kpropertiesdialog.cpp:2825
> +QMutexLocker locker(&d->mutex);
> +cacheChecksum(checksum, alg);
> +}

If the mutex is locked only around the cacheChecksum function I might as well 
put in back inside it

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added a comment.


  Won't using the blocking versions block the UI as well?... that's why I chose 
to use the non blocking versions and just update the UI once the work is 
finished by the other threads.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak updated this revision to Diff 24402.
petermajchrak added a comment.


  Screen out the cached algorithms beforehand

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=24401&id=24402

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak marked 2 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kpropertiesdialog.cpp:2798-2804
> You don't understand me, now you have a race condition. cachedChecksum reads 
> from cache while cacheChecksum writes. It's a race condition and you need 
> mutex here. When you add here you will not needed anymore in cacheChecksum.

I can add two critical sections, one for reading the cache and one for writing 
while the hash computation is not in either of them

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak updated this revision to Diff 24401.
petermajchrak added a comment.


  Fix mutex placement and add show cached checksum values when combobox value 
is changed

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=24390&id=24401

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kpropertiesdialog.cpp:2798-2804
> You don't understand me, now you have a race condition. cachedChecksum reads 
> from cache while cacheChecksum writes. It's a race condition and you need 
> mutex here. When you add here you will not needed anymore in cacheChecksum.

But adding a mutex for guarding the body of the mapper function will serialize 
the hash computations. The computations themselves are independent and 
parallelizable, only the synchronization points (cachedChecksum and 
cacheChecksum) are not.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kpropertiesdialog.cpp:3015
> Mutex is not needed here.

This function is called from the QtConcurrent mapper function and therefore can 
be called concurrently by more threads. QMap::insert is not thread safe for 
concurrent write operations as far as I know.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak marked an inline comment as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak updated this revision to Diff 24390.
petermajchrak added a comment.


  Capture item path by value

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=24389&id=24390

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-27 Thread Peter Majchrak
petermajchrak updated this revision to Diff 24389.
petermajchrak added a comment.


  Format algorithm values in combobox.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=24381&id=24389

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-26 Thread Peter Majchrak
petermajchrak marked 3 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-26 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-26 Thread Peter Majchrak
petermajchrak updated this revision to Diff 24381.
petermajchrak added a comment.


  Reverted back to the combobox based layout

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=23772&id=24381

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-21 Thread Peter Majchrak
petermajchrak added a comment.


  Ou, ok. Sorry, I didn't know that. Perfect, so combobox it is then.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-20 Thread Peter Majchrak
petermajchrak added a comment.


  I can revert it back to the combobox based layout. Don't we have some UX 
people that could take a look at this?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-20 Thread Peter Majchrak
petermajchrak added a comment.


  @anthonyfieroni, @elvisangelaccio, @colomar any progress? just want to get a 
status... this diff seems to be taking quite a long time to get reviewed after 
making changes...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-12-11 Thread Peter Majchrak
petermajchrak updated this revision to Diff 23772.
petermajchrak added a comment.


  Small code tweaks, nothing that changes functionality

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=22957&id=23772

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-11-26 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> anthonyfieroni wrote in kpropertiesdialog.cpp:2904-2906
> It's a private function, i can't image why it can be call with null pointers 
> :)

I am looking up the UI components based on name and I was getting some nullptrs 
until I found the right way to get them correctly from the layout. Not sure if 
they are needed but in case something in the layout changes (like a name of a 
button) it might pass in a nullptr

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-11-26 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-11-26 Thread Peter Majchrak
petermajchrak marked 4 inline comments as done.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-11-26 Thread Peter Majchrak
petermajchrak updated this revision to Diff 22957.
petermajchrak added a comment.


  I reverted back to the group box layout and parallelized hash calculations 
(removed the annoying recursive function).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=21535&id=22957

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio, #vdg, colomar
Cc: colomar, anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, 
#frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-11-26 Thread Peter Majchrak
petermajchrak added a comment.


  I am still not clear on what to do now. Should I revert it to be a lot of 
separate buttons (hidden in a collapsible group box) (as @elvisangelaccio said) 
or should I keep it in a dropdown (ok according to me and @anthonyfieroni)?... 
I'm going to fix some other stuff in the meantime. I also made this 
`verifyChecksumRecursive` function to verify the checksums serially (one after 
another)(because up to 2 algorithms can be candidates for certain checksum 
inputs) to not consume too much resources. I'm thinking of doing that in 
parallel instead. Do you think it's a good idea? Are there any guidelines for 
these situations?

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: anthonyfieroni, bcooksley, alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-29 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: bcooksley, alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-29 Thread Peter Majchrak
petermajchrak updated this revision to Diff 21535.
petermajchrak added a comment.


  Convert to only combobox based layout

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=21495&id=21535

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio
Cc: bcooksley, alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  I will also need help figuring out what config to use for storing the users 
last used hash algorithm from the combobox (if we go that route).

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: bcooksley, alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  A possible combobox layout
  F5454950: Screenshot_20171028_225633.png 


REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: bcooksley, alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  KIO repository

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: alexeymin, ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  It is kinda questionable as far as what are the most common hash algorithms. 
I most often use SHA512 for example. I think having only the combobox would be 
ok for me, but I'm not a UX person. So just let me know if you figure out 
anything else.

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  I'm going to give that a try.

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak updated this revision to Diff 21495.
petermajchrak added a comment.


  Place advanced algorithms into KCollapsibleGroupBox

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=21480&id=21495

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  I am new to KDE development and I want to use the KCollapsibleGroupBox but I 
don't know how. Are there any plugins for Qt Designer for KWidgetsAddons?

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: ngraham, elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added a comment.


  I agree that the layout is not optimal. I'm going to try to optimize it now.

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak, elvisangelaccio
Cc: elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak updated this revision to Diff 21480.
petermajchrak added a comment.


  Remove C++17 features

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D8536?vs=21478&id=21480

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak
Cc: elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in CMakeLists.txt:6
> We can't use this in frameworks (and the patch doesn't build for me), see 
> https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11

Ok. I was kinda hoping this would be ok, but I can see that it can be an issue 
of course. Going to fix it and update the diff. It provided nice structure 
binding syntax for std::pair. But that's just my code OCD...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak
Cc: elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak
Cc: elvisangelaccio, #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak edited the summary of this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

To: petermajchrak
Cc: #frameworks


D8536: Add more hashing algorithms to KPropertiesDialog

2017-10-28 Thread Peter Majchrak
petermajchrak created this revision.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: Frameworks.

REVISION SUMMARY
  I added most of the hashing algorithms to the dialog. I still need to figure 
out a better way to lay out the GUI but the code should be fine.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D8536

AFFECTED FILES
  CMakeLists.txt
  src/widgets/checksumswidget.ui
  src/widgets/kpropertiesdialog.cpp
  src/widgets/kpropertiesdialog_p.h

To: petermajchrak
Cc: #frameworks