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

Refactor and simplify color handling #257

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

Spudz76
Copy link
Contributor

@Spudz76 Spudz76 commented Mar 23, 2019

Refactor and simplify color handling

  • generate one version in color
  • degrade to non-color at broadcast once, as required
    • the ANSI sequences are too simple to keep double handling them, when they can be reliably filtered by a couple lines of code at the end instead

...instead of...

  • isColor ? X : Y nearly everywhere, and
  • X and Y are basically the identical string
    • did not see any need for differences where there were actual differences
      it doesn't matter if there are some apostrophes around the colored action letter for example,
      plus then the content is identical aside from the color which just makes sense

Also refactored all the color defines, both so they stack and build from single definitions, and all primitives of ANSI attribute control used elsewhere in code are now there (\x1B[ everywhere was UUUUUUgly)

MUCH easier to change/add/check things, also can generate color even over in other sections (RED_BOLD on the self-test fail for example - it will get stripped if noncolor output destination, yay!)

@Spudz76 Spudz76 force-pushed the dev-refactorColor branch 2 times, most recently from 53dbbb2 to 3fc33ac Compare March 23, 2019 23:41
…tead of isColor() checking everywhere and duplication)
@xmrig
Copy link
Owner

xmrig commented Mar 27, 2019

Thank you for this PR, I rewrote whole logging subsystem in evo branches for CPU miner and proxy, it include all your ideas and some code (copyright added to source files) with simplified log backends.

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