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

Coloured output #203

Closed
EpicWink opened this issue Mar 23, 2023 · 14 comments · Fixed by #433
Closed

Coloured output #203

EpicWink opened this issue Mar 23, 2023 · 14 comments · Fixed by #433
Labels
enhancement New feature or request

Comments

@EpicWink
Copy link

EpicWink commented Mar 23, 2023

Problem

A little difficult to distinguish between context, added, deleted, and modified properties in monochromatic output.

Proposal

Add option to have coloured output:

  • For YAML, JSON and text format, if the output is a TTY, using terminal escapes
  • For HTML, using CSS (ideally via classes)

Perhaps on by default (-color auto), with explicit enable/disable (disregard output not being a TTY for -color always).

Blue for context (endpoint method/paths, 'XXX changed'), red for deleted, orange/yellow for modified, and green for added.

Alternatives

Deal with no colours.

@reuvenharrison reuvenharrison added the enhancement New feature or request label Mar 23, 2023
@JSSAggie
Copy link

image

Would this add the ability to turn off the colors? If so I am all for it. We are pulling the breaking changes into our API spec pul requests and the color values in the error messages are not helping (:

@EpicWink
Copy link
Author

CI usually does not have stdout be a TTY, so my proposal would default to no colours in that case

@reuvenharrison
Copy link
Collaborator

@JSSAggie oasdiff automatically disables colored output when it detects piped output so you can easily remove the colors as follows:
oasdiff breaking spec1 spec2 | cat
or
oasdiff changelog spec1 spec2 | cat

@JSSAggie
Copy link

JSSAggie commented Oct 5, 2023

I still think the flag is valuable. We can pipe out to a json file in the meantime.

@reuvenharrison
Copy link
Collaborator

Hi @JSSAggie,
#386 will remove color codes from json and yaml output.
Does this address your requirement?

@JSSAggie
Copy link

Looks like that will do it. Thanks

@ls-jad-elkik
Copy link

It's unclear to me if the -color always suggestion was actually implemented in the linked PR.

I would find this helpful since in our CI we'd like to oasdiff breaking ... | tee artifacts/diff.txt to be able to process that txt file later on while still showing colors in the CI step itself. So the auto-detection of a TTY with no override is backfiring in our case.

@reuvenharrison
Copy link
Collaborator

Hi @ls-jad-elkik,
We haven't yet implemented -color always.
As of now oasdiff only adds ANSI Color Escape Sequences if the output is not piped.
I would recommend running oasdiff twice as a temporary workaround, once with output to the CI step and once again to store the artifact.

In preparation for a proper solution, could you clarify whether you expect artifacts/diff.txt to contain colors or not?

@ls-jad-elkik
Copy link

Hi @reuvenharrison,

We're going to be using the workaround. The idea was to maybe strip the color codes before storing the artifact, because along with colors the indentation is gone when tty detection kicks in. And indentation is also important to make it readable.

I don't think it's a very critical feature, but it's kind of standard to either have that --color=[auto|always|off] flag or COLOR=XXX env var or FORCE_COLOR env var, etc.

Thanks for the quick reply and clarification.

@reuvenharrison
Copy link
Collaborator

We will add --color=[auto|always|never]
Thanks for your input!

@reuvenharrison
Copy link
Collaborator

The latest release adds support for color=auto|always|never
Your feedback is welcome.

@EpicWink
Copy link
Author

EpicWink commented Dec 1, 2023

Ahh, I didn't specify which commands I would like to see coloured output in the original request. diff I think would also benefit from colours

@reuvenharrison
Copy link
Collaborator

Hi @EpicWink,
Which format of the diff report are you using? json, text, html, or yaml?
The json and yaml formats provide a low-level diff report which is not ideal for human-readers.
The diff report in text or html formats is easier to read.
But the best report for humans is the changelog.

It would be useful to understand this and also the general use-case: how you use the diff output?

By the way, you are welcome to join our slack community where we can discuss this privately:
https://join.slack.com/t/oasdiff/shared_invite/zt-1wvo7wois-ttncNBmyjyRXqBzyg~P6oA

Thanks,
Reuven

@EpicWink
Copy link
Author

EpicWink commented Dec 1, 2023 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants