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

[Feature]: configure whitespace error highlighting #45

Closed
evanrichter opened this issue Jul 17, 2023 · 4 comments
Closed

[Feature]: configure whitespace error highlighting #45

evanrichter opened this issue Jul 17, 2023 · 4 comments
Labels
enhancement New feature or request

Comments

@evanrichter
Copy link

evanrichter commented Jul 17, 2023

Description

git diff on the terminal highlights incorrect whitespace in the terminal. Example:

image

But in gex (tested with 0.5.0 using nix run nixpkgs#gex), this whitespace error isn't shown:

image

Additional Information

The git diff flag to use is --ws-error-highlight, and I think an appropriate value would be new.

wait, should this actually be a bug report?

It seems like running the git command doesn't read my user level git config either, for some reason, even when I set diff.wsErrorHighlight = new

@evanrichter evanrichter added the enhancement New feature or request label Jul 17, 2023
@Piturnah
Copy link
Owner

Piturnah commented Jul 17, 2023

It seems like running the git command doesn't read my user level git config either, for some reason, even when I set diff.wsErrorHighlight = new

My guess is git doesn't render the highlighting because it doesn't detect a TTY due to the pipe... I suspect this can be fixed super easily by just passing --color flag, but I wonder if there's a more general way to make the subprocess think it's talking to TTY

Thanks for opening this issue! I will look into it

Edit: when you referred to "running the git command" I interpreted this as using the : button to execute the command, but it just occurred to me that you might be referring to the underlying git diff that gex executes during a status refresh.

For the latter it is a reasonable solution to try just passing --color, but this won't be completely trivial as currently all the highlighting is done by gex after already being stripped from git's output (because of no-TTY), so this fix will require getting rid of that logic which sounds like a good thing but there are some potential problems

  1. If git doesn't reset its colouring for each line then this will cause problems when scrolling is considered, as the ANSI sequences to set the colours may be outside of the printed window
  2. In future I plan to possibly move all of the painting logic to the render module, because
    a) it makes more sense there and
    b) this will make it a lot easier and cleaner to resolve Line wrapping breaks the scrolling + non-ANSI truncation #37
    c) almost definitely will make painting simpler for Syntax highlighting in diffs #21

@evanrichter
Copy link
Author

when you referred to "running the git command" ...

ah, sorry, I was just talking about the main gex output. I noticed the way the diff is generated (here, for unstaged commits) is like:

let diff = git_process(&["diff", "--no-ext-diff"])?;

so my comment was about running git like this seems to ignore the config I explicitly set. The TTY detection theory makes sense

Piturnah added a commit that referenced this issue Jul 19, 2023
First checks the gex config, then if nothing is found it falls back on
the git config.

Close #45
@evanrichter
Copy link
Author

evanrichter commented Jul 20, 2023

oh sweet! can't wait to try this out :D

update: this is sick!!
image

@Piturnah
Copy link
Owner

Released in 0.6 🎉

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

No branches or pull requests

2 participants