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

Support numeric aware sorting on Windows and macOS #8363

Merged
merged 1 commit into from
Sep 8, 2022

Conversation

droidmonkey
Copy link
Member

Note: There is no std library way to do this so Linux is out of luck for now.

Screenshots

image

Testing strategy

Tested and mac and windows

Type of change

  • ✅ Bug fix (non-breaking change that fixes an issue)

@droidmonkey droidmonkey added user interface pr: backport pending Pull request yet to be backported to a previous release labels Aug 13, 2022
@droidmonkey droidmonkey added this to the v2.7.2 milestone Aug 13, 2022
@droidmonkey droidmonkey requested a review from phoerious August 13, 2022 21:25
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2022

Codecov Report

Merging #8363 (6d2f407) into develop (61f9221) will decrease coverage by 0.24%.
The diff coverage is 100.00%.

❗ Current head 6d2f407 differs from pull request most recent head c189573. Consider uploading reports for the commit c189573 to get more accurate results

@@             Coverage Diff             @@
##           develop    #8363      +/-   ##
===========================================
- Coverage    64.67%   64.43%   -0.24%     
===========================================
  Files          338      339       +1     
  Lines        43623    43740     +117     
===========================================
- Hits         28212    28182      -30     
- Misses       15411    15558     +147     
Impacted Files Coverage Δ
src/gui/SortFilterHideProxyModel.cpp 90.00% <100.00%> (+6.67%) ⬆️
src/gui/osutils/nixutils/NixUtils.cpp 33.53% <0.00%> (-9.47%) ⬇️
...rc/fdosecrets/widgets/SettingsWidgetFdoSecrets.cpp 56.06% <0.00%> (-3.03%) ⬇️
src/fdosecrets/dbus/DBusMgr.cpp 52.20% <0.00%> (-1.47%) ⬇️
src/core/FileWatcher.cpp 85.54% <0.00%> (-1.20%) ⬇️
src/format/OpVaultReaderSections.cpp 85.57% <0.00%> (-1.10%) ⬇️
src/cli/Show.cpp 97.30% <0.00%> (-0.62%) ⬇️
src/gui/Application.cpp 31.28% <0.00%> (-0.28%) ⬇️
src/core/Entry.cpp 83.78% <0.00%> (-0.20%) ⬇️
src/cli/Utils.cpp 75.11% <0.00%> (-0.20%) ⬇️
... and 5 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Member

@phoerious phoerious left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What speaks against using QCollator?

@droidmonkey
Copy link
Member Author

droidmonkey commented Aug 29, 2022

Oh great, I'll switch to that

Nevermind, despite the documentation it is absolutely worthless, unless we use icu on Linux. https://code.woboq.org/qt5/qtbase/src/corelib/text/qcollator_posix.cpp.html#58

@phoerious
Copy link
Member

You would still get rid of the Windows and macOS boilerplate code.

@droidmonkey
Copy link
Member Author

Yah sorry I meant to come back to this one.

* Fix #8356 - Qt does not enable numeric aware sorting when using locale sort. Extracted both Windows and macOS locale aware sorting code and added the appropriate numeric aware flag.

Note: There is no std library way to do this so Linux is out of luck for now.
@droidmonkey
Copy link
Member Author

ready for you

@droidmonkey droidmonkey closed this Sep 7, 2022
@droidmonkey droidmonkey reopened this Sep 7, 2022
@droidmonkey droidmonkey merged commit 4c1e5ec into develop Sep 8, 2022
@droidmonkey droidmonkey deleted the hotfix/numeric-sort branch September 8, 2022 10:47
@droidmonkey droidmonkey added pr: backported Pull request backported to previous release and removed pr: backport pending Pull request yet to be backported to a previous release labels Sep 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: backported Pull request backported to previous release user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

A way to change the alphabetical order
3 participants