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))

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

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?

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.

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.

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 :)

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?

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.

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.

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

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.

#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...).

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

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.

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.

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).

Agreed, will fix.

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.

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)

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.

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

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

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

What platform?

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

Ubuntu 13.04, xmonad.

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.

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.

@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.

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

Sure - will do shortly.

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.

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

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.

@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

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.