feat(shell): support no color mode

AvitalTamir / cyphernetes

A Kubernetes Query Language

https://cyphernet.es

Apache License 2.0

527 stars 11 forks source link

feat(shell): support no color mode #144

Closed naorpeled closed 2 weeks ago

naorpeled commented 3 weeks ago

TODO:

[x] make sure returned json objects are still formatted, just without color
[x] add and fix tests (TestSyntaxHighlighterPaint is the only one left to fix)
[x] cleanup

Linked issue

closes #131

AvitalTamir commented 3 weeks ago

Hey I know it's still in draft but I checked it out and looks cool! Works as advertised 😂

naorpeled commented 3 weeks ago

@AvitalTamir still need to fix some tests and add some tests, will try to get to it later today, worst case will do it tomorrow evening 🙏

Also, should I also adjust the shell-graph.go file to support no-color, I don't see any actual usages and not sure what I'm missing.

naorpeled commented 3 weeks ago

@AvitalTamir still got some issues to resolve, hopefully will be able to boost this significantly tomorrow 🙏

AvitalTamir commented 3 weeks ago

All good, let me know if you need any help bringing this to the finish line

naorpeled commented 3 weeks ago

All good, let me know if you need any help bringing this to the finish line

Actually if you'll have a bit of time to sync over Zoom tomorrow that'd be awesome

AvitalTamir commented 3 weeks ago

Let's do it

naorpeled commented 3 weeks ago

Let's do it

Would 18:30 work for you?

AvitalTamir commented 3 weeks ago

Yea, DMing you

naorpeled commented 3 weeks ago

@AvitalTamir the PR is ready for review, tried to test as many scenarios possible, hopefully I've caught all edge cases. FYI TestCyphernetesShellWithLogLevelFlag still isn't passing locally for some reason

let me know what you think 🙏

naorpeled commented 3 weeks ago

TODO (in a separate PR):

[ ] don't format AS as uppercase if part of a json path (e.g. bla.as becomes bla.AS) - this is the current behavior before my change.
[ ] support uppercasing and coloring the return keyword even if there's nothing after it (the other keywords work like that so I guess it'd be nice to have the same behavior there as well)

naorpeled commented 2 weeks ago

@naorpeled thank you so much for putting in the effort. Great work. Are we ok now with the tests in main_test.go or you still experiencing failures locally? Locally they pass for me, and I see CI is green as well. Let me know, if we're good I'll merge :)

They seem to be passing now, I think it was an issue with my local setup, did some cleanup and now it seems to be passing consistently.

I believe we can merge this now :)