-
Notifications
You must be signed in to change notification settings - Fork 30
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
added formatting helpers for balance (zcn,mzcn,uzcn,sas) #10 #225
Conversation
zcncore/zcnfmt.go
Outdated
case MZCN: | ||
return fmt.Sprintf("%.3f mZCN", float64(token)/1e6) | ||
} | ||
return fmt.Sprintf("%.3f ZCN", float64(token)/1e9) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to this issue here
0.0000000001 ZCN = 0.0000001 mZCN = 0.0001 µZCN = 1 sas
reverse of this
1 µZCN = 1e4 sas
1 mZCN = 1e7 sas
1 ZCN = 1e10 sas
But i see you have considered only 1e9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed the testFormat() function is wrong. I think the confusion lies is Saswatas not being a 3rd power of decimals like usual thousand separators. It should use 1e4, 1e7 and 1e10. Saswatas shouldn't really be displayed if we set threshold for displaying uzcn to 4 decimals, e.g. 1 sas = 0.0001 µZCN
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to clarify, the format chosen by autoformat is a separate layer to the amount of raw zcn being shown.
That is, there could be user preference to show amounts as zcn down to 0.01 ZCN, then below that e.g. 0.0099 would be shown as 9.9mZCN.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@NoSkillGuy, @sculptex thanks for pointing it out. I"ll fix the order as 1 zcn = 1e10 sas.
@sculptex, I guess I could add additional functions to format them into html as well as for CLI with color support. For a client side UI code, it makes sense to implement the same logic separately. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks numerically correct now for values but only on mobile so can't check fully.
Since sas are lowest denomination, it doesn't make sense to have decimal places on those?
There is also scope on ZCN if value is large, e.g. 1,000,000ZCN you really aren't bothered with showing 3 decimal places. There could either be a straight cut off above a certain value, or decrease decimal places,
e.g.
<1,000 3 decimals
=1,000 2 decimals
=10,000 1 decimal
=100,000 no decimals
Removed decimal formatting from SAS, but left the 3 digit formatting on ZCN (unit) for now. I'll merge this PR and tag with a new version so that we can update the dependency on zwalletcli and zboxcli. |
This PR includes formatting helpers used to format balance (ZCN) to string for CLI applications. It addresses the issue raised here: 0chain/zwalletcli#10