Ported from https://github.com/bitcoin/bitcoin/pull/17877.
Since Qt 5.5 there are means to register an enum type with the meta-object system (such enum still lacks an ability to interact with QSettings::setValue() and QSettings::value() without defined stream operators).
In order to reduce global namespace polluting and to force strong type checking, this PR makes BitcoinUnits::Unit a scoped enum (typedef BitcoinUnits::Unit BitcoinUnit;).
No behavior change.
tACK b109ae6
I just noticed one practical downside of this separate repo; having to add another remote: git remote add hebasto-gui git@github.com:hebasto/gui.git
git fetch https://github.com/hebasto/gui/ 200814-unit, which works without a remote
265+{
266+ switch (num) {
267+ case 3: return BitcoinUnits::SAT;
268+ case 2: return BitcoinUnits::uBTC;
269+ case 1: return BitcoinUnits::mBTC;
270+ default: return BitcoinUnits::BTC;
77edf1bd0149bd2625dff563f3bd4032a12a2233
Add case for 0 and an assert(false) in default?
247@@ -248,3 +248,39 @@ CAmount BitcoinUnits::maxMoney()
248 {
249 return MAX_MONEY;
250 }
251+
252+namespace {
253+qint8 ToQuint8(BitcoinUnit unit)
77edf1bd0149bd2625dff563f3bd4032a12a2233
ToQint8?
70@@ -70,9 +71,15 @@ void OptionsModel::Init(bool resetSettings)
71 fMinimizeOnClose = settings.value("fMinimizeOnClose").toBool();
72
73 // Display
74- if (!settings.contains("nDisplayUnit"))
75- settings.setValue("nDisplayUnit", BitcoinUnits::BTC);
77edf1bd0149bd2625dff563f3bd4032a12a2233
Maybe it should migrate the old setting key if it exists?
@promag I was thinking about your suggestion, and I failed to find a safe and short (about 1..2 lines of code) solution. OTOH, adjusting this option requires only two clicks.
Therefore, I’m going to leave it as is, leaving a room for improvements in follow ups.
507@@ -501,12 +508,11 @@ bool OptionsModel::setData(const QModelIndex & index, const QVariant & value, in
508 /** Updates current unit in memory, settings and emits displayUnitChanged(newUnit) signal */
509 void OptionsModel::setDisplayUnit(const QVariant &value)
510 {
511- if (!value.isNull())
512- {
513+ if (!value.isNull()) {
77edf1bd0149bd2625dff563f3bd4032a12a2233
nit, could skip if value == m_display_unit?
Code review ACK 7e636c7fe129afff8ff248cdd6b55e59445f1e0e with some comments.
Will test and play around.
This change improves type safety.
Since BitcoinUnits::Unit became a scoped enum, BitcoinUnits::valid
function is no longer needed.
Co-authored-by: João Barbosa <joao.paulo.barbosa@gmail.com>
Code Review ACK 8eb1a34937990012d661d262a029e33c8cb67776
looks like a reasonable cleanup
note: I found this PR with a git script that emails you when someone touches a part of the codebase you’ve previously touched (via git-contacts and git-blame). I recommend using this trick to find potential reviewers: 😉
0(for commit in $(git log --format=%H origin/master..FEATUREBRANCH); do git-contacts ${commit}^-; done) | sort | uniq -c | sort -nr
1 5 Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
2 4 Wladimir J. van der Laan <laanwj@gmail.com>
3 2 William Casarin <jb55@jb55.com>
4 2 Russell Yanofsky <russ@yanofsky.org>
5 1 João Barbosa <joao.paulo.barbosa@gmail.com>
6 1 GreatSock <greatsock@protonmail.com>
🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.
I have a rebased version here: https://github.com/bitcoin-core/gui/compare/master...jb55:units-scoped-enum
I can submit a PR if you want me to take this over @hebasto
I have a rebased version here: master…jb55:units-scoped-enum
I can submit a PR if you want me to take this over @hebasto @jb55
Thank you! I’ll appreciate if you do that.