kubectl-neat
kubectl-neat copied to clipboard
itaysk
add syntax highlighting for output
fixes #39
- [x] Add code for syntax highlight for top level / root command
- [x] Add test for syntax highlight for top level / root command
- [x] Add code for syntax highlight for get sub command
- [x] Add test for syntax highlight for get sub command
Consider this a draft. Of course we can first discuss and make any necessary changes and then merge :)
With yaml, the output looks nice, but with JSON, it doesn't look that great actually, even though JSON is only a subset of YAML. I mean, the braces and the spaces have some highlighting. I have to check if this is really expected
And I haven't added any unit or E2E test. I was wondering if we should test it and what to test and how to do it.
Should we add just E2E test? Since it's more CLI output related. I can't think of anything related to unit test, but I can see that there are some unit tests around output. We could do that.
Any thoughts on testing?
And I haven't added any unit or E2E test. I was wondering if we should test it and what to test and how to do it.
Should we add just E2E test? Since it's more CLI output related. I can't think of anything related to unit test, but I can see that there are some unit tests around output. We could do that.
Any thoughts on testing?
- Check that the option has impact (different output) for JSON & YAML with/without this option
- Check that error code is consistent with and without this option
- Check that writing it twice pass correctly
- Check that the first line of both YAML & JSON output contains color sequence
- Check the color of YAML by reread the output and checking that all color sequences are in place (It is problematic for JSON since it will fail for numbers)
Could you explain more about what points 2, 3 and 5 mean? I didn't understand them properly
Could you explain more about what points 2, 3 and 5 mean? I didn't understand them properly
2 - Ensure that if the command failed without color, it is failing with color also 3 - Call command with "-c -c" and see that it is working 5 - Call command with color, read the output and compare it to what should be the output (with color character sequences)
What's the benefit of 4 (single line check) if 5 does a complete check?
What's the benefit of 4 (single line check) if 5 does a complete check?
4 is working for JSON also, I agree that for YAML is redundant if you will do also 5.
About 2 - Why would the error code change because of this option? Only when highlighting has some issues (error), there will be a non-zero exit code
About 2 - Why would the error code change because of this option? Only when highlighting has some issues (error), there will be a non-zero exit code
It shouldn't - this is reason why need such test (I had such bug 20 years ago and it was really nagging)
Makes sense. I'll add it.
But I was just wondering, so, the test strategy is -
For every feature flag that's present, test if the usage of any flag causes the exit code to change in an unexpected manner? Doesn't that sound tedious? Or it could be done as some sort of parameterized test, but still that would be a lot of combinations depending on the number of flags and possible error situations
This project doesn't have much flags, so all good. Just wondering in general 😅
This project doesn't have much flags, so all good. Just wondering in general 😅
I think that this is question for @itaysk and should be documented somewhere.
Cool. I thought you would have an answer since you suggested the strategy
Another thing is, I have only written code for the root command. I'm yet to understand and try out the "get" sub command. If it needs the color, we need to add for it too (looks like it does need it)
Do we surely need the full output check? I think we can trust the library and just check that the option works - that is, invokes the library, by just checking the first line
Do we surely need the full output check? I think we can trust the library and just check that the option works - that is, invokes the library, by just checking the first line
I am not sure this why I split it to two different tests.
You can review and let me know how it looks and if there are any more changes to be done :)
@karuppiah7890 The library that this code rely on, which in turn depended on another library which isn't working well on Windows. I didn't check it personally, but it is something to check (manually at least) :-(
@AssafKatz3 cool, I'll check it out! Currently we still don't have support for windows however (no artifacts) but yeah, I'll be sure to check it out
Apparently the library may not work on Windows - https://github.com/itaysk/kubectl-neat/pull/52#issuecomment-752850500 . I'm not working on this PR now, so I'll close it. I hope new contributors will have an opportunity to work on it and also check support for windows based on #6 , #53
I'm gonna be checking about https://github.com/alecthomas/chroma library if that's okay.
cc @itaysk