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

all: make logs a bit easier on the eye to digest #22665

Merged
merged 5 commits into from
Apr 15, 2021

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Apr 14, 2021

This PR is up for debate, but the idea is that numbers displayed in the log lines are starting to get unwieldy. With state counts reaching soon 1 billion, it's just impossible to see at a glance what the number order of magnitudes are. This PR adds the thousandth separators for numbers > 99999. Numbers smaller are probably clearer without added symbols.

Now, it probably would take a bit of getting used to to see that many commas in the output, but I'd guess it might be worthwhile.

The PR also replaces the ellipse (three dots) unicode character from the logs with two ... This will result in the log package unquoting all the hashes (the quotes were caused by the non-ASCII byte).

Screenshot from 2021-04-14 17-49-46

(the image above didn't have the hash change in yet)

Final fancy algo courtesy of Felix.

@karalabe karalabe added this to the 1.10.3 milestone Apr 15, 2021
@karalabe karalabe merged commit 1e20734 into ethereum:master Apr 15, 2021
@fjl
Copy link
Contributor

fjl commented Apr 15, 2021

You really should've waited for review on this, the big.Int case was broken. Fixed in #22679.

atif-konasl pushed a commit to frozeman/pandora-execution-engine that referenced this pull request Oct 15, 2021
* all: add thousandths separators for big numbers on log messages

* p2p/sentry: drop accidental file

* common, log: add fast number formatter

* common, eth/protocols/snap: simplifty fancy num types

* log: handle nil big ints
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants