Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore directories-first order of passwords tree view on non-Mac platforms #475

Merged
merged 4 commits into from
Sep 23, 2019
Merged

Restore directories-first order of passwords tree view on non-Mac platforms #475

merged 4 commits into from
Sep 23, 2019

Conversation

maciejsszmigiero
Copy link
Contributor

Since commit b4dc9e6 ("Auto update CHANGELOG and sorting of treeview")
directories are no longer listed first in the tree view of passwords.

This is because a simple sort of the tree view widget (as enabled by the
aforementioned commit) does its work by sorting the backing
StoreModel (QSortFilterProxyModel), which in turn does just a simple
lexicographical order sort on the path of each its item.

Before that commit the sort was done by QFileSystemModel via
QFileSystemModelSorter, which always places its directories first on
non-Mac platforms.

Unfortunately, QFileSystemModelSorter is an internal Qt helper class, so we
can't just use it directly, we need to open-code a bit of logic from
QFileSystemModelSorter::compareNodes() into StoreModel::lessThan() to
restore the old behavior.

This PR also contains 3 small improvements around the modified code (in
separate commits).

When we are setting the source model in StoreModel::setModelAndStore() set
it also for StoreModel base class to make the extra setSourceModel() call
unnecessary.
…tforms

Since commit b4dc9e6 ("Auto update CHANGELOG and sorting of treeview")
directories are no longer listed first in the tree view of passwords.

This is because a simple sort of the tree view widget (as enabled by the
aforementioned commit) does its work by sorting the backing
StoreModel (QSortFilterProxyModel), which in turn does just a simple
lexicographical order sort on the path of each its item.

Before that commit the sort was done by QFileSystemModel via
QFileSystemModelSorter, which always places its directories first on
non-Mac platforms.

Unfortunately, QFileSystemModelSorter is an internal Qt helper class, so we
can't just use it directly, we need to open-code a bit of logic from
QFileSystemModelSorter::compareNodes() into StoreModel::lessThan() to
restore the old behavior.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 6.938% when pulling a06be7f on maciejsszmigiero:directories-first-order-fix into f9f9fba on IJHack:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 6.938% when pulling a06be7f on maciejsszmigiero:directories-first-order-fix into f9f9fba on IJHack:master.

@codecov
Copy link

codecov bot commented Sep 21, 2019

Codecov Report

Merging #475 into master will decrease coverage by 0.02%.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff            @@
##           master   #475      +/-   ##
========================================
- Coverage    7.32%   7.3%   -0.03%     
========================================
  Files          44     44              
  Lines        2798   2806       +8     
========================================
  Hits          205    205              
- Misses       2593   2601       +8
Impacted Files Coverage Δ
src/storemodel.h 0% <ø> (ø) ⬆️
src/mainwindow.cpp 0% <0%> (ø) ⬆️
src/storemodel.cpp 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f9f9fba...a06be7f. Read the comment docs.

@annejan annejan merged commit 9e73083 into IJHack:master Sep 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants