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

[Qt] fix coincontrol sort issue #9185

Merged
merged 2 commits into from Nov 22, 2016

Conversation

@jonasschnelli
Copy link
Member

commented Nov 18, 2016

Reported by @gmaxwell
The sort order for the amount column is lexicographical resulting in a order like "1 10 2 20 200 3 ...".

The attempts of this PR is to fix this.

@@ -721,11 +730,11 @@ void CoinControlDialog::updateView()

// amount
itemOutput->setText(COLUMN_AMOUNT, BitcoinUnits::format(nDisplayUnit, out.tx->vout[out.i].nValue));
itemOutput->setText(COLUMN_AMOUNT_INT64, strPad(QString::number(out.tx->vout[out.i].nValue), 15, " ")); // padding so that sorting works correctly
itemOutput->setData(COLUMN_AMOUNT_INT64, Qt::DisplayRole, QVariant((qlonglong)out.tx->vout[out.i].nValue)); // padding so that sorting works correctly

This comment has been minimized.

Copy link
@laanwj

laanwj Nov 18, 2016

Member

Let's get rid of the hidden _INT64 columns completely, they should no longer be necessary.

This comment has been minimized.

Copy link
@laanwj

laanwj Nov 18, 2016

Member

Also: function strPad can go after this.

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 18, 2016

Working on a patch

@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 18, 2016

Please cherry-pick the top commit from: https://github.com/laanwj/bitcoin/tree/2016_11_trol_cleanup into this. I've done some further cleanups and fixes:

  • Do sorting for date, amount and confirmations column as longlong, not unsigned longlong.
  • Use UserRole to store our own data for sorting. This makes it treated as ancillary data prevents it from being displayed.
  • Get rid of getMappedColumn strPad - these are no longer necessary.
  • Get rid of hidden _INT64 columns.
  • Start column enumeration from 0 (otherwise values are undefined).
[Qt] Clean up and fix coincontrol tree widget handling
- Do sorting for date, amount and confirmations column as longlong, not
  unsigned longlong.
- Use `UserRole` to store our own data. This makes it treated as
  ancillary data prevents it from being displayed.
- Get rid of `getMappedColumn` `strPad` - these are no longer necessary.
- Get rid of hidden `_INT64` columns.
- Start enumeration from 0 (otherwise values are undefined).
@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Nov 20, 2016

Thanks @laanwj!
Cherry picked your 4231032

@laanwj
laanwj approved these changes Nov 21, 2016
@laanwj

This comment has been minimized.

Copy link
Member

commented Nov 21, 2016

@gmaxwell Can you please test this in your environment and confirm whether the problem is fixed?

@jonasschnelli

This comment has been minimized.

Copy link
Member Author

commented Nov 21, 2016

Tested ACK (Ubuntu) after @laanwj commit.
Binaries: https://bitcoin.jonasschnelli.ch/pulls/9185/

Waiting for @gmaxwell's re-test

bildschirmfoto 2016-11-21 um 13 55 27

@gmaxwell

This comment has been minimized.

Copy link
Contributor

commented Nov 22, 2016

tested ACK!

@jonasschnelli jonasschnelli merged commit 4231032 into bitcoin:master Nov 22, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
jonasschnelli added a commit that referenced this pull request Nov 22, 2016
Merge #9185: [Qt] fix coincontrol sort issue
4231032 [Qt] Clean up and fix coincontrol tree widget handling (Wladimir J. van der Laan)
76af4eb [Qt] fix coincontrol sort issue (Jonas Schnelli)
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
luke-jr added a commit to luke-jr/bitcoin that referenced this pull request Dec 2, 2016
[Qt] Clean up and fix coincontrol tree widget handling
- Do sorting for date, amount and confirmations column as longlong, not
  unsigned longlong.
- Use `UserRole` to store our own data. This makes it treated as
  ancillary data prevents it from being displayed.
- Get rid of `getMappedColumn` `strPad` - these are no longer necessary.
- Get rid of hidden `_INT64` columns.
- Start enumeration from 0 (otherwise values are undefined).

Github-Pull: bitcoin#9185
Rebased-From: 4231032
codablock added a commit to codablock/dash that referenced this pull request Jan 15, 2018
Merge bitcoin#9185: [Qt] fix coincontrol sort issue
4231032 [Qt] Clean up and fix coincontrol tree widget handling (Wladimir J. van der Laan)
76af4eb [Qt] fix coincontrol sort issue (Jonas Schnelli)
andvgal added a commit to energicryptocurrency/energi that referenced this pull request Jan 6, 2019
Merge bitcoin#9185: [Qt] fix coincontrol sort issue
4231032 [Qt] Clean up and fix coincontrol tree widget handling (Wladimir J. van der Laan)
76af4eb [Qt] fix coincontrol sort issue (Jonas Schnelli)
CryptoCentric added a commit to absolute-community/absolute that referenced this pull request Feb 24, 2019
Merge bitcoin#9185: [Qt] fix coincontrol sort issue
4231032 [Qt] Clean up and fix coincontrol tree widget handling (Wladimir J. van der Laan)
76af4eb [Qt] fix coincontrol sort issue (Jonas Schnelli)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.