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

Ansi colors #35

Merged
merged 3 commits into from
May 3, 2022
Merged

Ansi colors #35

merged 3 commits into from
May 3, 2022

Conversation

NathanBaulch
Copy link
Contributor

Colors! This branch also includes my changes in #34 since colors are particularly useful for differentiating between different series in the same plot.
AnsiRainbow

@coveralls
Copy link

coveralls commented Apr 12, 2022

Pull Request Test Coverage Report for Build 2263993454

  • 78 of 81 (96.3%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.05%) to 95.597%

Changes Missing Coverage Covered Lines Changed/Added Lines %
color.go 13 16 81.25%
Totals Coverage Status
Change from base Build 2259956222: -0.05%
Covered Lines: 304
Relevant Lines: 318

💛 - Coveralls

@guptarohit
Copy link
Owner

Thanks, @NathanBaulch for your time to contribute to this change, it was a long time due. 🎉 🍰

I was thinking if we could also add flags for this color support in CLI utility it would be really great addon!

@NathanBaulch
Copy link
Contributor Author

Good idea, I went with -l since all the other letters in "color" are taken.

@guptarohit guptarohit force-pushed the master branch 6 times, most recently from 228c545 to 5deec54 Compare May 2, 2022 19:25
@guptarohit
Copy link
Owner

guptarohit commented May 3, 2022

Hey @NathanBaulch, I noticed while plotting the first value i.e. ┼ , it's also changing the color of y-axis for that value, is this how you expected it to be?
image

I'd like this to be AxisColor, like:
image

@NathanBaulch
Copy link
Contributor Author

is this how you expected it to be?

It was intentional yes, but I'm not too fussed either way. I wonder what kroitor does?

@guptarohit
Copy link
Owner

It was intentional yes, but I'm not too fussed either way. I wonder what kroitor does?

I just checked, looks like he's also making color after y-axis
Screenshot from 2022-05-03 17-09-50

@guptarohit
Copy link
Owner

Hey @NathanBaulch, could you please review the changes once, then I'll merge these changes

cmd/asciigraph/main.go Outdated Show resolved Hide resolved
@NathanBaulch
Copy link
Contributor Author

Looks good @guptarohit, just my comment above about the new cmd color flag errors all being the same.

@NathanBaulch
Copy link
Contributor Author

FWIW, the color names I went with are the standard SVG/CSS ones, as listed here: https://www.w3.org/TR/SVG11/types.html#ColorKeywords
In case you wanted a more "official" list rather than pointing people to color.go here.

@guptarohit
Copy link
Owner

cool, it make sense to use this link instead 👍 🎉 Thanks :)

@guptarohit guptarohit merged commit 748c9ad into guptarohit:master May 3, 2022
@NathanBaulch NathanBaulch deleted the ansi-colors branch May 6, 2022 02:08
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.

3 participants