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

cli: highlight "Error:"/"Warning:"/"Hint:" headings #3359

Merged
merged 5 commits into from
Mar 25, 2024

Conversation

yuja
Copy link
Contributor

@yuja yuja commented Mar 25, 2024

Checklist

If applicable:

  • I have updated CHANGELOG.md
  • I have updated the documentation (README.md, docs/, demos/)
  • I have updated the config schema (cli/src/config-schema.json)
  • I have added tests to cover my changes

yuja added 5 commits March 25, 2024 20:28
This will be used to label "Error: " heading and content differently. I want
to see an error message in the default (white) color because it's easier to
read, but I still want to highlight the "Error: " heading.

We can achieve that without introducing new wrapper, but the resulting code
would look something like "writeln!(ui.error("Error: ")?, ..)?", and it would
get messier if the caller had to suppress io::Error.
The existing .hint() method is renamed to .hint_no_heading() to clarify that
it's not the default choice to print a hint. I'll add .hint_default() later,
which will be the shorthand for .hint_with_heading("Hint: ").
…dings

The lowercase "warning: " is unified to "Warning: " as it is the jj's
convention afaik.

The _default() suffix could be dropped from these methods, but it's probably
better to break the existing codebase for the moment. Otherwise, the caller
might do writeln!(ui.warning(), "Warning: ..").
Copy link
Member

@martinvonz martinvonz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I haven't looked at the output to see how I like it but I've wanted to make the same change myself before.

@yuja
Copy link
Contributor Author

yuja commented Mar 25, 2024

I haven't looked at the output to see how I like it but I've wanted to make the same change myself before.

The output doesn't look so different at this point. I'll send a color scheme change separately.

@yuja yuja merged commit bd3d930 into jj-vcs:main Mar 25, 2024
16 checks passed
@yuja yuja deleted the push-slyuvzwwrqyz branch March 25, 2024 15:28
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