Implement SI-style (thin space) thoudands separator #4167

pull roybadami wants to merge 5 commits into bitcoin:master from roybadami:master changing 10 files +202 −47
  1. roybadami commented at 11:05 PM on May 9, 2014: contributor

    Personally, I find long strings of digits (either side of the decimal point) hard to read.... To the right of the decimal point, it's OK as long as you really only care as to the first few..... But when you have multiple leading zeros 0.001 and 0.0001 are easy to confuse (but no big loss)

    But switching to milli- or micro- and 1000000 versus 100000 is a big deal.

    IMHO the only sane option is SI-style (i.e. a thin space as thousands separator, both left and right of the decimal point) because:

    • given the huge variation in punctuation marks used as decimal separators and thousands separators, anything else is confusing at best and ambiguous at worst
    • there's really no other widely used convention for thousands separators to the right of the decimal point

    This pull request adds thin spaces to formatted BTC amounts (both sides of the decimal point), but

    Thin spaces are avoided in the following cases:

    • Copying from the transaction view (I and I'm sure countless others rely on being able to cut and paste BTC values into spreadsheets)
    • Transaction exports (for similar reasons)

    (BTW, i never even looked at Qt before yesterday and this ended up involving somewhat more non-trivial Qt code than I anticipated- so this really should be reviewed by someone who knows Qt)

    ETA: Are there other cases where we should avoid thousands separators? (Or alternatively should the code avoid them by default and only insert them in certain cases - and if so in which cases?)

  2. roybadami commented at 11:50 PM on May 9, 2014: contributor

    BTW, this change also makes copying unconfirmed transactions consistent. (Previously, copying from the context menu copied without the square brackets, but copying using Ctrl+C copied with the square brackets. Now it's consistentsly copied without square brackets (or thousands separators))

  3. leofidus commented at 12:49 AM on May 10, 2014: none

    I support the idea of using thin spaces (and have advocated it previously, just didn't get around to making a pull request).

    For consistency, you might want to change the descriptions of µBTC and mBTC to use thin spaces too: https://github.com/roybadami/bitcoin/blob/master/src/qt/bitcoinunits.cpp#L53-L54

  4. laanwj commented at 6:36 AM on May 10, 2014: member

    Does this need any special precautions for parsing? What happens if you copy an amount, thin spaces and al, into the amount widget?

    Also when copy-pasting using double-click, it will see an amount with spaces in it as multiple units, probably? Or are these spaces special?

  5. laanwj commented at 6:39 AM on May 10, 2014: member

    I still think it makes more sense to use locale-specific number formatting (implemented in #3893) than invent a convention that inserts spaces in numbers (edit: okay, not invent, according to the internet it's somewhat common as a unambiguous thousands separator). On the other hand this has a lot less impact on the code.

    Inserting separators in the decimals after the decimal point is also strange. There is no locale that does that as far as I know.

  6. in src/qt/transactiontablemodel.cpp:None in 265de82fe5 outdated
     569 | @@ -570,6 +570,8 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const
     570 |          return rec->status.countsForBalance;
     571 |      case FormattedAmountRole:
     572 |          return formatTxAmount(rec, false);
     573 | +    case FormattedAmountWithoutThousandsRole:
    


    laanwj commented at 7:36 AM on May 10, 2014:

    What about a more general 'CopyRole' (or ExportRole), and make it valid for all columns? Then you wouldn't have to make an exception in the view code for a certain column.

  7. roybadami commented at 7:40 AM on May 10, 2014: contributor

    Separators after the point is absolutely 100% standard practice in SI notation. It's pretty much universal in scientific fields. (Pick up any physics textbook printed within the last 50 years and you'll see what I mean)

    It's true that, outside of scientific fields such conventions never developed, but that's largely because in mainstream use you rarely have very many digits to the right of the decimal point.

  8. roybadami commented at 7:49 AM on May 10, 2014: contributor

    Re a locale-specific approach, I'm neutral on that point - mainly I went down this route because the comments in BitcoinUnits::format state that we don't want to do that :)

  9. laanwj commented at 7:52 AM on May 10, 2014: member

    I'm really not certain on this. Both ways have long lists of advantages and disadvantages, and I don't want a long shed-painting discussion. I don't want to make this decision at all.

    Any idea what other wallets do?

  10. roybadami commented at 8:01 AM on May 10, 2014: contributor

    Maybe we need a preference to control this :-(

    Bitcoin Wallet for Android: no separators. Armory: no separators to the right of the decimal point (don't know about to the left as it doesn't support units smaller than bitcoin yet).

    Really, most people probably don't complain about lack of thousands separators to the right of the decimal because most people don't know they exist (although how you're supposed to distinguish 0.00001 from 0.000001 without them is beyond me). We don't have that problem yet, but I see people confuse 0.001 and 0.0001 often enough. Maybe this is just more fuel for the argument for a units change though.

    But if the community really does move to 'bits' then people are going to complain about the lack of them to the left of the decimal. Lots of people will have millions of bits, and knowing whether they have 10000000 bits or 1000000 bits or 100000 bits without having to count the zeros is important.

  11. laanwj commented at 8:06 AM on May 10, 2014: member

    We all agree that decimal separators are useful. If it was up to me, we'd have them already in 0.5.0. The problem is, in the Bitcoin community no one can ever agree on how to do something, so we eventually end up with no decision at all after a very long discussion.

    BTW on a more technical level: Do you know what happens if a value (with thin spaces as thousands separators) is pasted into the bitcoin amount widgets? They need to be able to cope with this.

  12. roybadami commented at 8:08 AM on May 10, 2014: contributor

    Re pasting, no - I need to take a look at that.

  13. roybadami commented at 8:12 AM on May 10, 2014: contributor

    Parsing formatted numbers is going to be a bit of a can of worms if you also want to support locale-specific numbers. You need to strip out thousands separators but not the decimal separator. Remember that 1,000 and 1.000 can mean the opposite of what you expect, depending which country you're in.

  14. laanwj commented at 8:13 AM on May 10, 2014: member

    #3893 actually implements such parsing based on the locale. But no need to worry about that here. We'll either use this or locale specific numbers, not both. I'm reluctantly starting to see the advantages of this, although I'm also sure a lot of people will complain (WHY DIDN'T YOU JUST...).

  15. roybadami commented at 8:16 AM on May 10, 2014: contributor

    Well, we could always make it a preference (SI-style, locale-specific, or none). Then all we have to argue about is the default :)

  16. laanwj commented at 8:21 AM on May 10, 2014: member

    I tried to go that way in #3893 but it may result in confusion. People will be using amounts formatted in this style in communication. Also internationally. So if different users have selected different options... And yes - locale-specific formatting has the same drawback.

  17. roybadami commented at 8:53 AM on May 10, 2014: contributor

    I had a look at pasting - right now pasting anything with (thin) spaces isn't allowed as it fails validation.

    Obviously we can fix this, but is there really a use case for allowing it? I think I prefer the approach of trying to prevent formatted values leaking into the clipboard in the first place. Parsing is a slippery slope - what if I paste '1000 mBTC' for instance?

    Also, re double click, that already doesn't do the right thing at the moment (it doesn't select across dot) so this change doesn't really make it any worse.

  18. laanwj commented at 8:59 AM on May 10, 2014: member

    Yes - that should be fixed. It's very annoying if a website displays an amount in this format, for example, and it isn't accepted. I've been very annoyed at this with my bank, which puts spaces in between some identifiers (for example, the IBAN) but subsequently fails to recognize them in that format, so you have paste it into some intermediate place to manually remove the spaces.

    You cannot avoid these values from leaking into the clipboard. People will copy the balance from the overview page, for example, or from the request payment window to an email. Hacking every single place to do something else on copy/pasting is not very elegant, either (or even realistic).

  19. roybadami commented at 9:21 AM on May 10, 2014: contributor

    Agreed, will fix.

  20. roybadami commented at 1:02 PM on May 10, 2014: contributor

    I've fixed the spin box to display and accept values with spaces, and also fixed the confirmation message box not to wrap values.

    My biggest reservation at the moment is that, on my machine (Mac) thin spaces really aren't as thin as I'd like. In most contexts they do seem to display slightly narrower than a standard space, but only very slightly. In message boxes they seem to display identically to ordinary spaces.

    Maybe it's a font issue, not sure.

  21. laanwj commented at 1:29 PM on May 10, 2014: member

    Great!

    It seems that thin spaces are supposed to be half as wide as a real space. There are even thinner spaces: https://en.wikipedia.org/wiki/Space_%28punctuation%29#Spaces_in_Unicode (SIX-PER-EM and HAIR)

  22. roybadami commented at 4:09 PM on May 10, 2014: contributor

    Ok, changed from U+2009 THIN SPACE to U+200A HAIR SPACE which I think looks a bit better.

    Qt seems to have a bug whereby the thin spaces in the HTML message box get displayed as normal space. I experimented with various workarounds, but it's hard to find one that's both supported by Qt's HTML engine and doesn't break the nowrap behaviour of the containing SPAN element.

    Only solution I could find was to display the space in a smaller font size, using an absolute font size. 6pt looks about right to me.

    Of course, it will need checking for suitable visual results on other platforms than Mac, since different font engines and default fonts might mean it needs tweaking differently for Windows and Linux.

  23. roybadami commented at 4:52 PM on May 10, 2014: contributor

    Re a more general CopyRole - the way the code is currently structured, it needs a separate role for each column. I could have just edited the existing FormattedAmountRole though rather than adding a new one, but I thought it better to leave FormattedAmountRole as it was (even though it's no longer used after this change), in case it's needed in the future.

    I agree that FormattedAmountWithoutThousands could better be called ExportFormattedAmountRole though

  24. maaku commented at 4:55 PM on May 10, 2014: contributor

    This is better than what we currently have, but my preference would be for locale-sensitive printing and parsing in the Qt wallet.

  25. sipa commented at 12:51 AM on May 11, 2014: member

    With the current code, the separation is barely noticable imho: spacesep

  26. roybadami commented at 1:16 AM on May 11, 2014: contributor

    What platform?

  27. roybadami commented at 1:23 AM on May 11, 2014: contributor

    Although I did feel that on my Mac thin space was too wide but hair space a little narrower than ideal, it wasn't quite as narrow as your screenshot. I think.

    Unfortunately these are ultimately just characters from a font so the actual width is subject to the whims of the font designer.

    The difficulty of getting the visual aspects right here is starting to give me reservations as to whether this is a sensible approach

  28. sipa commented at 9:19 AM on May 11, 2014: member

    Ubuntu 13.04, xmonad.

  29. roybadami commented at 1:34 PM on May 11, 2014: contributor

    screen shot 2014-05-11 at 14 28 46

    I think it's marginally better on my system (OS X 10.9.2) but I agree it's probably a bit too narrow. I'll see if I can experiment with other types of spaces.

  30. roybadami commented at 2:01 PM on May 11, 2014: contributor

    drak: because it's standard practice to do so in every scientific field. You seriously want to have to count digits in order to distinguish 0.0000001 BTC from 0.00000001 BTC? (Of course, you could fix this by writing 0.1 µBTC or 0.01 µBTC). And 1.000 and 1,000 can both mean "exactly one" or "one thousand" depending on country, so are useless in any international context.

    As locale-specifc versus SI-style formatting, I don't feel very strongly on the issue.

  31. leofidus commented at 4:20 PM on May 11, 2014: none

    In my opinion it's even enough spacing on sipa's system. This isn't about the user consciously noticing the space but more about guiding his eye. That function is fulfilled perfectly for me, I have no problem assessing the numbers without counting the digits.

    Even better test cases would be amounts with many identical digits, as that's what's especially problematic to asses right now (e.g. 1 BTC in µBTC, 1 Satoshi in BTC). @drak BTC is already borrowing SI prefixes, I don't see why it can't borrow the only existing convention that makes large amounts of digits after the decimal point readable?

  32. sipa commented at 4:45 PM on May 11, 2014: member

    Disagree... I need conscious effort to see the grouping. Without it, it just looks misaligned.

  33. roybadami commented at 5:34 PM on May 11, 2014: contributor

    drak: I'm not a scientist, I'm a Linux sysadmin (although I did study physical sciences to university level).

    I really think the comparison with fiat currency (like accounting software or ATMs) is misguided because:

    • Fiat currency pretty much invariably works either in whole numbers or with exactly two decimal places. This pretty much avoids the ambiguity, and also avoids the requirement for grouping to the right of the decimal point
    • With the exception of the Eurozone, fiat currency is largely country-specific anyway. If I'm trying to send fiat to someone in another country or am requesting fiat payment from someone in another country then whether we write dots or commas really is the least of our problems

    As for exchanging accounts, spreadsheets or other financial documents across international borders, this isn't something the general public does -- and accounting professionals who work in this area will be used to the issues.

    We're not special flowers here; the requirement to be able to accurately and precisely communicate numerical values across international boundaries -- where those values might be very large or very small, and might contain a large number of significant digits -- is not new to Bitcoin. Smart people have already thought about this problem and solved it decades ago. But if the Bitcoin community wants to reinvent the wheel, be my guest.

    I watch with some bemusement the reactions to suggesting we use this standard and widely adopted convention -- but I'm particularly interested in the reaction to grouping the digits to the right of the decimal point, which has ranged from surprise to active hostility. What this really proves to me is that, even amongst a highly technical set of users, most people really aren't used to dealing with numbers with more than a couple of digits to the right of the decimal point on a day-to-day basis. More than anything, this convinces me that µBTC is the only way to go. (And I say this as someone who started out not that long ago as a units change skeptic.)

  34. roybadami commented at 5:42 PM on May 11, 2014: contributor

    I should add that I wasn't aware of Wladimir's pull request (#3893) when I submitted mine. I chose to use the SI convention specifically because BitcoinUnits::format contains the comment:

    // Note: not using straight sprintf here because we do NOT want
    // localized number formatting.
    
  35. roybadami commented at 5:53 PM on May 11, 2014: contributor

    sipa: I've reverted to U+2009 THIN SPACE

    On reflection I think this looks right. (It was the QMessageBox HTML issue which originally took me down the path of thinking thin spaces are too wide. But that appears to be a Qt bug, which I've since worked around.)

  36. roybadami commented at 5:55 PM on May 11, 2014: contributor

    screen shot 2014-05-11 at 18 54 44

  37. gmaxwell commented at 5:59 PM on May 11, 2014: contributor

    Visual misalignment could perhaps be avoied by aligning numbers around the period. This would also make numbers of similar sizes look more similar. Alternatively, and perhaps superiority, all digits could be printed at all times, even if they are zero— which would guarantee alignment and perhaps prevent some misreading. (just some thoughts, I am not a UI guru)

  38. roybadami commented at 6:00 PM on May 11, 2014: contributor

    screen shot 2014-05-11 at 18 59 12

  39. roybadami commented at 6:00 PM on May 11, 2014: contributor

    screen shot 2014-05-11 at 19 00 37

  40. roybadami commented at 6:10 PM on May 11, 2014: contributor

    gmaxwell: yeah, one or other of those would seem to be a good idea - not sure which I prefer

  41. roybadami commented at 6:20 PM on May 11, 2014: contributor

    BTW, just for the record:

    TODO: Review all remaining calls to BitcoinUnits::formatWithUnit to determine whether the output is used in a plain text context or an HTML context (and replace with BtcoinUnits::formatHtmlWithUnit in the latter case). Hopefully there aren't instances where the result could be used in either context.

    (I'm not going to do bother to do this until/unless there's any consensus to proceed with this)

  42. sipa commented at 6:28 PM on May 11, 2014: member

    Now I have a different concern. Without doing effort, this looks like just very big number to me. The fact that one of the spacings has a dot in it doesn't stand out.

    spacesep

  43. luke-jr commented at 6:33 PM on May 11, 2014: member

    Make the font size slightly smaller for the fractional part?

  44. roybadami commented at 6:45 PM on May 11, 2014: contributor

    sipa: Yeah, I know what you mean. But I'm not sure replacing the spaces with commas would be better in that respect - if anything it might be worse. Myabe gmaxwell's idea's would help, though.

    luke-jr: interesting idea. I think this approach works with mBTC or µBTC where the digits in the small font are, in a very real sense, insignificant. But with BTC you're not really getting into the territory where the small digits are representing insignifcant amounts of money

  45. luke-jr commented at 6:48 PM on May 11, 2014: member

    75% opacity text then?

  46. roybadami commented at 6:54 PM on May 11, 2014: contributor

    Not sure. Also I worry that either smaller font or transparency will compromise readability. It's not like we're starting with a particularly large font.

  47. roybadami commented at 7:00 PM on May 11, 2014: contributor

    Just picked up an old textbook from my university days, and interestingly at least in this particular book they seem to use spaces to the right of the decimal point if there are five or more digits, but they don't if there are only four.

    Now I'm wondering if that's the correct convention - or perhaps more generally you prefer a final group of four digits to a single hanging digit (I couldn't immediately find any examples of exactly 7 or 10 digits to the right of the decimal point so I can't tell)

    leofidus: what do you think? Do you know where this is documented?

    EDIT: Actually, I think it is the single hanging digits that kept looking wrong to me. Part of why I kept trying to reduce the space width, I think, as I mistook the reason it looked wrong

    EDIT: It appears this book does the same to the left of the decimal point. It writes '1000' without a thousands separator, but writes '10 000' with one.

  48. roybadami commented at 7:07 PM on May 11, 2014: contributor

    This looks a lot better to me now

    screen shot 2014-05-11 at 20 06 09

    I'd love to have confirmation. And particularly to know what the right thing is to do when you have e.g. seven figures to the right of the point (I couldn't immediately find an example, but I suspect now the right thing is a group of three followed by a group of four)

  49. roybadami commented at 7:17 PM on May 11, 2014: contributor

    Not committing yet until someone can research whether the correct rule is to always prefer a final group of four to a final single digit or whether the rule should be to always place a thin space every three digits, but only if you have five or more digits.

    If the former is right (I suspect it is) fix is to replace r_size with r_size-1 in bitcoinunits.cpp and bitcoinamountfield.cpp

  50. leofidus commented at 10:22 PM on May 11, 2014: none

    @roybadami the International Bureau of Weights and Measures says in the SI brochure "Following the 9th CGPM (1948, Resolution 7) and the 22nd CGPM (2003, Resolution 10), for numbers with many digits the digits may be divided into groups of three by a thin space, in order to facilitate reading. Neither dots nor commas are inserted in the spaces between groups of three. However, when there are only four digits before or after the decimal marker, it is customary not to use a space to isolate a single digit."

    So you're right. It's 1000, but 10 000 and 1 000 000. Similarly 0.0001 but 0.000 01 and 0.000 000 1.

  51. roybadami commented at 10:49 PM on May 11, 2014: contributor

    Oh, excellent, thanks leofidus. Actually, I was wrong -- I had assumed maybe in the seven digit case we would write 0.000 0001

    EDIT: Although looking at my version written down it looks wrong.

    I'll fix to implement to correct version.

  52. roybadami commented at 11:10 PM on May 11, 2014: contributor

    leofidus: while we're at it, do you know what's the correct spacing between the value and the unit - is it space or thin space? (I skimmed the document you linked but couldn't find the right section.)

  53. leofidus commented at 11:19 PM on May 11, 2014: none

    I thought it was a thin space, but the SI brochure seems to only mention a space (section 5.3.3). Another source (German Wikipedia) recommends a thin non-breaking space. The fact that units are treated as mathematical entities also suggests thin spaces (if I remember mathematical typesetting right).

  54. roybadami commented at 11:25 PM on May 11, 2014: contributor

    Could I suggest that people who feel strongly one way or the other on this issue identify which locale(s) they're in/have lived in? (EDIT: And what the number formatting conventions are in those locales, where we're not all likely to know already.)

    FTR I'm in boring old en_GB. (Well, boring at least in the sense that we don't have any particularly exotic number formatting conventions.)

    I'm interested because those in en_GB or en_US saying "use locale-specific number formatting" counts for little since it won't have much impact on us. I'd like to here what people living in different locales want -- particularly people living in locales (such as de_DE) that use dot as the preferred thousands separator.

    I don't know, but strongly suspect, that people in de_DE (German German) have to contend with a mishmash of localized and non-localized software. So that "1.000" might mean "one thousand" from a localized app, or it might mean "exactly one" from a non-localized app (that always uses a dot as the decimal point, regardless of locale).

    Case in point, Bitcoin Core is currently exactly such an app that always used dot, regardless of locale.

    So it's not at all clear to me that de_DE residents will even thank us for switching from the current "1000" to a localized "1.000", since the latter probably can't be correctly interpretted without context.

    But I'd love to know what people in those locales really think.

  55. leofidus commented at 11:32 PM on May 11, 2014: none

    I'm a de_DE user who appreciates the unambiguity of unlocalized numbers. When I copy a price of 1,000 mBTC from a US merchant to my bitcoin client, I would be very annoyed if local aware parsing would suddenly read it as 1 mBTC.

    I'm used to switching between the number formats constantly, but BTC provides less clues the usual, as somebody else here already said.

  56. roybadami commented at 11:55 PM on May 11, 2014: contributor

    Not sure what's going on, but pretty sure this wasn't caused by deciding not to insert thousands separators into four digit numbers :)

    1:46:22 10 AbstractBlockChain.add: Failed to verify block: com.google.bitcoin.core.VerificationException: Block had too many Signature Operations at com.google.bitcoin.core.Block.checkSigOps(Block.java:673) at com.google.bitcoin.core.Block.verifyTransactions(Block.java:791) at com.google.bitcoin.core.AbstractBlockChain.add(AbstractBlockChain.java:363) at com.google.bitcoin.core.AbstractBlockChain.add(AbstractBlockChain.java:239) at com.google.bitcoin.core.BitcoindComparisonTool.main(BitcoindComparisonTool.java:218)

  57. laanwj commented at 7:08 AM on May 12, 2014: member

    @gmaxwell Aligning on the '.' would be the best solution, but it's harder to do, it would require at least a custom cell formatter for the transaction table to always center around the dot. That's why it was never done. The table formatter also doesn't support HTML formatting so the 'hidden digit' approach doesn't work either. That approach could be used for the balance display on the overview page, although you'd need a fixed-width font for it to work.

  58. roybadami commented at 7:45 AM on May 12, 2014: contributor

    @laanwj: Most fonts use monospaced digits anyway, I think. As for hidden digits, U+2007 FIGURE SPACE looks like it was invented specifically for this purpose.

  59. laanwj commented at 10:36 AM on May 12, 2014: member

    Ok, if fonts use monospaced digits and there exists a digit-wide space, it sounds like it's suddently pretty easy to do. I did not know that.

  60. roybadami commented at 9:00 PM on May 12, 2014: contributor

    Ok, so I was a bit worried about the convention that I and @leofidus had been researching, where if you have exactly four digits on either side of the decimal point, you omit the thousands separator and just write a group of four. This would mess up digit alignment in tabular data -- and it would also make it much harder to line up the decimal points.

    Fortunately, looking at actual real life data, people don't actually like printing tables that look horribly misaligned. I found two ways in which this was tackled in real life tabular data to the right of the decimal point (presumably the same approaches can be used to the left).

    • In a table where most values had four decimal places, but the odd one had five, they simply suspended the use of thin spaces
    0.99955
    0.9956
    0.9812
    0.9703
    0.9672
    0.9721
    
    • In a table where there were lots of numbers with five decimal places, but the odd one with four, they simply suspended the rule special casing four digits:
    1.747 56
    1.772 67
    1.638 05
    1.641 32
    2.519 39
    2.408
    2.221 24
    4.171 9
    

    So it seems that we can be pragmatic and do what makes sense given the nature of the data. It seems eminently doable to do something sensible with this here.

  61. laanwj commented at 5:01 AM on May 13, 2014: member

    @roybadami Even scientists can be pragmatic sometimes! But indeed, it makes sense to simply group per three, at least in the overview page and transaction overview, as having the numbers below each other align up is more important here. It's also easier for people to learn this (for many) new convention of grouping digits after the dot if done consistently per three.

    To be honest I'm still a bit divided on the whole concept of grouping digits after the dot. Maybe people that want that should just switch to mBTC/muBTC to get the long slew of digits before the dot. But it might grow on me...

  62. Implement SI-style (thin space) thoudands separator 7007402956
  63. roybadami commented at 1:55 PM on May 17, 2014: contributor

    This was getting messy, so I've squashed to a single commit. Implemented alignment in the overview and transaction table. It gets kinda messy though, when combined with the use of [square brackets] to show unconfirmed transactions. Maybe it's good enough, though? (Perhaps I could float the closing bracket to the left of the figure spaces in the transaction view?)

  64. roybadami commented at 2:01 PM on May 17, 2014: contributor

    screen shot 2014-05-17 at 15 00 49 screen shot 2014-05-17 at 15 00 55 screen shot 2014-05-17 at 15 01 07

  65. roybadami closed this on May 17, 2014

  66. roybadami reopened this on May 17, 2014

  67. laanwj commented at 12:07 PM on May 28, 2014: member

    I agree it's impossible to get alignment to work properly as long as brackets can appear as well as numbers. Choosing something else than brackets to show that the amount doesn't count towards the spendable balance would be fine with me, BTW, if clear, I'm not wedded to them :) Edit: But it's fine to leave that for another pull... I think it looks fine like this.

  68. roybadami commented at 12:21 PM on May 28, 2014: contributor

    I'm not sure what would be better though, but I agree, the misalignment isn't that horrible.

    I think I'll float the right bracket to the left of the spaces (in the case where there is no unit) and then leave it at that.

  69. laanwj commented at 9:12 AM on June 23, 2014: member

    I've tested this a bit and think it looks great in uBTC, reasonable in mBTC, and plain strange in BTC. So much empty space... I'm starting to think we should just pad with zeros.

    ubtc Microbtc: looks great

    btc BTC: too much empty space makes it hard to relate things. Having whitespace between digits after the dot, combined with padding makes for a lot of whitespace.

    btc_null BTC: now with zeros for padding. Looks much better to me.

  70. leofidus commented at 5:01 PM on June 23, 2014: none

    I agree that padding with zeros looks much better

  71. roybadami commented at 7:18 PM on June 23, 2014: contributor

    On balance, I think showing a consistent number of decimal places is better anyway. But that may be because of my physics background again - in a scientific context the number of decimal places quoted conveys the precision of the quantity. 0.001 BTC and 0.00100000 BTC mean slightly different things (the former may have been rounded to the nearest millibitcoin; the latter clearly is exact down to the last satoshi)

  72. laanwj commented at 3:10 PM on June 24, 2014: member

    @roybadami Yes, agreed. Let's always show full precision. It also removes all doubt as to how subdividable bitcoins are, no matter what unit they have selected.

  73. laanwj commented at 6:00 AM on July 3, 2014: member

    @roybadami I'd like to merge this -- are you going to update to do padding with zeros instead of spaces?

  74. roybadami commented at 2:01 AM on July 5, 2014: contributor

    Sure - will do shortly.

  75. Show bitcoin quantities with full precision, even in the presence of trailing zeros 2e4fee2ac4
  76. roybadami commented at 8:07 PM on July 7, 2014: contributor

    Done. Note that, ideally, the following should still also be done (sorry, I don't have time to look at this right now):

    "TODO: Review all remaining calls to BitcoinUnits::formatWithUnit to determine whether the output is used in a plain text context or an HTML context (and replace with BtcoinUnits::formatHtmlWithUnit in the latter case). Hopefully there aren't instances where the result could be used in either context."

    Using formatWithUnit in an HTML context risks wrapping quantities at the thousands separator. More subtly, it also results in a standard space rather than a thin space, due to a bug in Qt's XML whitespace canonicalisation.

  77. Merge remote-tracking branch 'upstream/master'
    Conflicts:
    	src/qt/overviewpage.cpp
    	src/qt/transactiondesc.cpp
    96df327834
  78. Remove unused fAlign argument from BitcoinUnits::format and friends f7d70c603f
  79. Add comments re BitcoinUnits::formatWithUnit/formatHtmlWithUnit 7149499fd8
  80. roybadami commented at 9:31 PM on July 7, 2014: contributor

    Fixed merge conflict, removed now-redundant arguments to some methods, and added a comment re the TODO.

    NB: this builds and runs for me, but is very lightly tested

  81. BitcoinPullTester commented at 9:51 PM on July 7, 2014: none

    Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/p4167_7149499fd85d5adea23c9c3057944c3f2f69a2d2/ for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

  82. roybadami commented at 6:12 PM on July 8, 2014: contributor

    @laanwj I just realised I might have misinterpreted what you were asking for.

    What I've done is to completely remove the code that strips trailing zeros, so that all bitcoin quantities, everywhere, are to full precision.

    The alternative would have been to retain the fAlign flag that I introduced to control padding with figure spaces, and to only show full precision in those contexts where alilgnment is requested (currently the overview page and the transaction history). In that case, sums of bitcoin elsewhere (e.g. in confirmation dialogs) would remain as now, showing as few as two decimal places.

    Let me know if it was the latter you wanted and I'll revert the appropriate parts of the changes.

    roy

  83. laanwj commented at 11:01 AM on July 18, 2014: member

    What I've done is to completely remove the code that strips trailing zeros, so that all bitcoin quantities, everywhere, are to full precision.

    Sounds good to me, to be consistent here.

  84. laanwj merged this on Jul 18, 2014
  85. laanwj closed this on Jul 18, 2014

  86. laanwj referenced this in commit 40d2d69223 on Jul 18, 2014
  87. laanwj referenced this in commit d59d3e6aac on Sep 8, 2014
  88. laanwj referenced this in commit 20b46e7367 on Sep 8, 2014
  89. laanwj referenced this in commit a95b1199db on Sep 8, 2014
  90. rebroad referenced this in commit 92d290c027 on Sep 11, 2014
  91. fanquake referenced this in commit da279fe0ee on Jan 5, 2020
  92. sidhujag referenced this in commit 641989f7e7 on Jan 5, 2020
  93. reddink referenced this in commit 5ebcc5da07 on May 27, 2020
  94. sidhujag referenced this in commit ea2d42601c on Nov 10, 2020
  95. DrahtBot locked this on Sep 8, 2021

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-13 18:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me