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

🐛 git add -p uncoloured in Windows Terminal in Git for Windows 2.38+ #1318

Closed
astiob opened this issue Feb 28, 2023 · 32 comments
Closed

🐛 git add -p uncoloured in Windows Terminal in Git for Windows 2.38+ #1318

astiob opened this issue Feb 28, 2023 · 32 comments

Comments

@astiob
Copy link

astiob commented Feb 28, 2023

Another strange Windows thing, sorry!

I upgraded Git for Windows from some older version (I can’t see which it was any more, sorry) to 2.39.2 (the latest available via winget) (git --version says “2.39.2.windows.1”), and now git add -p shows a plain diff that is completely uncoloured within hunks. The workaround from #1114 helps:

Screenshot of various Git invocations in Windows Terminal: coloured foreground if invoked without options, uncoloured added and removed lines if invoked with Delta as a diff filter, and full Delta colouring if invoked with Delta as a diff filter and with the built-in interactive add turned off.

but it is supposed to be unnecessary since Git 2.38. Moreover, in mintty (Git Bash) it works fine without the workaround:

Screenshot of mintty showing an interactive add fully coloured by Delta.

Stranger still, it works in Windows Terminal if I use MSYS2’s git instead of Git for Windows, but then I get strange colours in non-interactive diffs:

Screenshot of Windows Terminal showing MSYS2 git 2.39.2 display fully colourized output but a weird grey background behind the bottom border of a hunk header in git diff.

Maybe there’s another bug in Git’s new interactive add, or maybe there’s one in Delta, or maybe even in Windows Terminal. What can I do to debug this?

I don’t see any visible difference (with Delta disabled) between the “native” and “non-native” interactive adds, and if I set the diffFilter to tee myfile, the files I get with Delta on and off are identical. And unlike in the Git 2.37 issue reports, I don’t get any error message: the output is just uncoloured; but then again, I tried installing 2.37 just to check, and I still didn’t see any error message (maybe it depended on the exact diff or repo structure).

Using Windows Terminal 1.16.10262.0 on Windows 11 22H2 (the current version of both).

@ybc37
Copy link

ybc37 commented Mar 27, 2023

Sadly, the workaround (#1114 (comment)) doesn't work anymore with Git 2.40, because add.interactive.useBuiltin was removed:

[...]
Similarly, Git 2.40 retired the legacy implementation of git add --interactive, which also began as a Shell script and was re-introduced as a native builtin back in version 2.26, supporting both the new and old implementation behind an experimental add.interactive.useBuiltin configuration.

Since that default has been “true” since version 2.37, the Git project has decided that it is time to get rid of the now-legacy implementation entirely, marking the end of another years-long effort to improve Git’s performance and reduce the footprint of legacy scripts.

https://github.blog/2023-03-13-highlights-from-git-2-40/

@eduardo-sm
Copy link

I've found this thread while searching for an issue with git add -p (version 2.40.1.windows.1).

Using git add -p <file> makes git interactive mode unresponsive and not even ctrl-c can exit the process.

image

I tracked the issue to this configuration in git:

[interactive]
  diffFilter = delta --color-only

It seems to work fine only with git's built-in tty terminal but the hanging is reproducible with Windows Terminal, Alacritty and conhost.exe.

I tried with another computer that has git v2.39.2.windows.1 and it doesn't reproduce the issue.

@ap-axentia
Copy link

ap-axentia commented May 3, 2023

Also searched around and found this, and wanted to report my findings.

I see the same thing in Windows Terminal (1.16.10262.0, Windows 11) and git version 2.40.1.windows.1, where the terminal hangs when doing git add -p:

image

It does not hang in the Git Bash external application, but the colors are still incorrect:

image

git diff works as expected, and delta colors it correctly, both with and without --color-only

image

git add -p works as expected with diff-highlight, i.e.:

[interactive]
    diffFilter = perl /path/to/diff-highlight

image

@khellang
Copy link

Seeing the same here 😢

  • Delta 0.15.1
  • Git 2.40.1.windows.1
  • Windows Terminal 1.16.10261.0

@dandavison
Copy link
Owner

dandavison commented May 31, 2023

Hi all, problems on Windows are either due to the terminal emulator, or to an old version of less (the pager) being picked up. Note that Git bundles a newish version of less but Delta will not necessarily pick up the version of less bundled by git. That said, I don't think less is involved in git add -p, right?

So, is it the terminal emulator that is the problem? Can you run it in something else, like Alacritty, and report back on whether that works? EDIT: Sorry I see now that the hanging was reproduced in Alacritty. Hm!

Is it 24-bit colors that are causing the problems? Can you try true-color = never?

cc @helmesjo

@dandavison
Copy link
Owner

I wonder whether the git-for-windows mailing list might be worth trying https://groups.google.com/g/git-for-windows.

@eduardo-sm
Copy link

eduardo-sm commented Jun 6, 2023

@dandavison I've checked some of the things you mentioned.

Just for clarification about the terminal emulators:

  • Alacritty v0.11.0 (d23330a) reproduces the issue.
    It also reproduces with:
  • Windows Terminal v1.16.10262.0
  • Conhost

Git v2.40.1.windows.1 comes with less v629. I've updated less manually with a more recent binary (v633) and the issue still reproduces.

For comparison I've checked the less version that comes with Ubuntu 22.04 (WSL) and that's v590. In Ubuntu the issue is not reproducible.

I don't think less is involved in git add -p, right?

The issue is with interactive mode but I don't know if git uses less for it.

@dandavison Is here where I need to add --true-color never, right?

[interactive]
diffFilter = delta --color-only

Adding --true-color never didn't help but gave me a grey background for the text. Without --true-color never I don't get a background color in the text.

With --true-color never
image

Without --true-color never
image

I also found that you can escape the hang with ctrl-z (I imagine being windows it kills the process as it can't be recovered with bg or fg).

Delta version 0.16.5

@dandavison
Copy link
Owner

Wow, nice investigation @eduardo-sm, thanks for that. (I don't have a Windows machine to test on). So this seems quite non-obvious. This seems like a possible clue / starting point:

git add -p works as expected with diff-highlight, i.e.:

[interactive]
    diffFilter = perl /path/to/diff-highlight

I have no reason to think this would work but I'm curious whether if you created a perl (or bash) script that simply passes stdin on to delta, whether that might (for some reason!) work? So I guess with bash that could simply be

[interactive]
    diffFilter = bash /tmp/delta.sh

where /tmp/delta.sh contains the line

delta --color-only

@dandavison
Copy link
Owner

dandavison commented Jun 6, 2023

Can you also try

[interactive]
    diffFilter = delta --color-only --paging=never

@eduardo-sm
Copy link

Here are the results.

Wrapping delta in a script and use it as follows still reproduces the issue.

[interactive]
    diffFilter = bash /tmp/delta.sh

Although I'm not the one who uses diff-highlight. I wonder if it is this one.

For the second attempt with --paging=never

[interactive]
    diffFilter = delta --color-only --paging=never

It still reproduces the issue.

On a final note, I noticed that using ctrl-z without delta (without the hang) won't kill the interactive add and instead it will add ^Z.
image
Hopefully this provides more hints about the issue.

@astiob
Copy link
Author

astiob commented Jun 6, 2023

Just a note (as I haven’t found the time to re-test this on my end yet and am still using Git 2.39 instead):

I also found that you can escape the hang with ctrl-z (I imagine being windows it kills the process as it can't be recovered with bg or fg).

On a final note, I noticed that using ctrl-z without delta (without the hang) won't kill the interactive add and instead it will add ^Z.

In Windows consoles, Ctrl-Z doesn’t act like a suspend signal (I don’t think one exists at all) but commonly signifies end-of-file, when Microsoft’s C library is used to read the input. Typically (e. g. in Python’s REPL), you have to type Ctrl-Z + Enter on a line of its own where in Unix you’d press Ctrl-D, and the Ctrl-Z stays visible on screen as ^Z. But I imagine some applications (e. g. I’m seeing this in uutils coreutils’ cat) might catch the Ctrl-Z and exit immediately. (uutils also handles a double Ctrl-Z in the middle of a line but exits with status 3… 🤔 But this is probably irrelevant.)

@eduardo-sm
Copy link

I found something new. Looking for interactive.diffFilter I found this issue #178.
I tried setting less as described there

[interactive]
  diffFilter = less -RFX

Depending on the version of less I got different results.

  • Less from git v629
    • Color works. It doesn't hang.
  • Less from MSYS2 v633
    • Color works. It doesn't hang.
  • Less from scoop v633.
    • Color works. It hangs similar to delta.

I'm not entirely sure on the differences but this may help to identify the issue.

@dandavison
Copy link
Owner

OK, so if you configure delta to use those 3 less versions (e.g. --pager=/path/to/less), does the pattern of hangs with delta match what you saw with less?

(Also does --paging=never get rid of the hang?)

@eduardo-sm
Copy link

No, delta hangs with the 3 versions of less --pager=/path/to/less or with --paging=never.

@ap-axentia
Copy link

ap-axentia commented Jun 7, 2023

Regarding diff-highlight and perl. Yes it is https://github.com/git/git/tree/master/contrib/diff-highlight I am using, I have been using that for many years before I found the much better delta :)

I now tried to wrap delta --color-only in both a bash script

delta --color-only

and a perl script

system("delta", "--color-only");

And both of them behaved just as if placing delta --color-only directly in diffFilter, i.e. no better, still hanging.

The point I wanted to make with diff-highlight was mostly that it is still possible to redirect and recolor in at least some way with the newer git, so the interactive add was not breaking all redirection at least.

@dandavison
Copy link
Owner

dandavison commented Jun 7, 2023

So the current conclusion seems to be that the Windows version of Git is, for git add -p, somehow invoking the child process in a way that it hangs when delta is involved, despite the fact that it can invoke a perl script without problems. And the pager process which delta is capable of invoking is not the problem.

Could you try

[interactive]
    diffFilter = bat

? bat is, like delta, a Rust executable, and they are written in a similar way to a certain extent. I've tested that and it works for me on MacOS (you shouldn't get the "Your filter must maintain a one-to-one correspondence" error because bat emits un-decorated output when it is writing to a pipe, as is the case when invoked under git add -p).

@eduardo-sm
Copy link

eduardo-sm commented Jun 7, 2023

@dandavison I tried with bat and git shows colors and there is not hanging.

bat v0.23.0 (871abd2)

@dandavison
Copy link
Owner

Hmm. @th1000s this is a bit mysterious -- any ideas? Could it be something to do with sysinfo on Windows?

@eduardo-sm
Copy link

eduardo-sm commented Jun 12, 2023

I just tried using winpty and it works (git add -p doesn't hang) BUT you don't get any color output. Adding --true-color=never makes the output with grey background.

Tested usage

winpty git add -p [file]

Using winpty in the diffFilter gives the error of "Your filter must maintain a one-to-one correspondence".

@th1000s
Copy link
Collaborator

th1000s commented Jun 12, 2023

Could this be Windows 11 specific? Using 10 and conhost/cmd.exe or the VSCode builtin terminal (which starts powershell) I could not reproduce this with git 2.41.0 from (gitforwindows.org), nor with the Portable Git 2.40 one.

@eduardo-sm
Copy link

@th1000s I've tried on windows 10 and the issue is reproducible there.

What I meant by conhost is conhost running the bash shell from git for windows. The issue is not reproducible in cmd.exe or powershell (windows powershell or powershell core).

Out of curiosity I tried with the build-in terminal from vscode and it also reproduces the issue using the bash shell from git for windows.

@th1000s
Copy link
Collaborator

th1000s commented Jun 13, 2023

I see, bash inside conhost is quite a QoL improvement, and I can reproduce it now. When the git add -p . Stage..? prompt shows up, delta and any pager have already exited, which means the terminal is left in an unexpected state.

The good news: It's not delta. The bad news: This is triggered by a simple std::io::stdout().write_all(&buffer), i.e. a naive cat written in Rust [1]. And it is the write, which can be shown as the following also has the issue: Commiting a file foo with abc in it, then wanting to stage an appended def line using this program [2] - which does not read from stdin, it only writes.

What does not hang for me:

  • A cat subprocess: std::process::Command::new("cat").spawn().expect("cat spawning failed");
  • A shellscript #!/bin/bash with target/debug/delta.exe --color-only, which seems to contradict ap-axentia - could you try to "launder" the output in the shell script by piping the delta output into cat, i.e. delta --color-only | cat?
  • Using the git binary from the portable version: ../PortableGit-241/bin/git.exe add -p . (also v2.40).

I expected --paging=always to help because then delta writes into the pager (less), but to no avail.

How do PortableGit binaries work for you, and could somebody check if [1] reproduces this?

1:

use std::io::{Write, Read};

fn main() -> std::io::Result<()> {
    let mut reader = std::io::stdin();
    let mut buffer = Vec::new();
    reader.read_to_end(&mut buffer)?;
    std::io::stdout().write_all(&buffer)?;
    Ok(())
}

2:

fn main() -> std::io::Result<()> {
    let data = "diff --git a/foo b/foo
index 8baef1b..5f5521f 100644
--- a/foo
+++ b/foo
@@ -1 +1,2 @@
 abc
+def";
    println!("{}", data);
    // std::io::stdout().write_all(&data[..])?; // OR (+ Write import, and data = b"diff..."): also hangs
    Ok(())
}

@th1000s
Copy link
Collaborator

th1000s commented Jun 13, 2023

It very much looks related to this git-for-windows / MYS2 issue git-for-windows/git#4422 and it seems the dev is still wrangling with it: git-for-windows/git#4464.

@eduardo-sm
Copy link

@th1000s I tried using a script delta.sh. I had mine with shebang #!/usr/bin/env bash and it was reproducing the hang but changing it to #!/bin/bash works without having to add cat (I can add cat but it works regardless if it is included). That's my bad, I probably did that by instinct but in this case I don't know why #!/usr/bin/env bash behaves different.

Note that using the script with the correct shebang doesn't show colors:
image

Full delta.sh script:

#!/bin/bash
delta --color-only

@ap-axentia
Copy link

ap-axentia commented Jun 15, 2023

I tried some more in Git Bash in Windows Terminal, not sure if it is still helpful or not.

  • diffFilter = bat works as expected (no hang and colors the output correctly) (bat 0.23.0 (871abd2))
  • diffFilter = cat works (no hang and does produce colored output) (cat (GNU coreutils) 8.32)
  • diffFilter = ~/testenv/bash-delta.sh with delta --color-only or delta --color-only | cat does NOT hang, but it gets gray background.
  • diffFilter = ~/testenv/bash-delta.sh with delta --color-only or delta --color-only | cat AND a shebang (either of the two suggested above) does hang.
  • diffFilter = bash ~/testenv/bash-delta.sh with delta --color-only or delta --color-only | cat does hang.

So for me it seems like invoking some form of bash hangs it?
My bash version: GNU bash, version 5.2.15(1)-release (x86_64-pc-msys)

@eduardo-sm
Copy link

Interesting. I used an absolute path for the diffFilter key in the config. Changing the path to use ~ make it reproduce the hang.

Results:

  diffFilter = C:/Users/eduardo/AppData/Local/Temp/delta.sh

The shebang #!/bin/bash does not reproduce the hang.

  diffFilter = ~/AppData/Local/Temp/delta.sh

The shebang #!/bin/bash reproduces the hang.

@ap-axentia
Copy link

Huh, yeah I see the same thing, tested with absolute path now.

@th1000s
Copy link
Collaborator

th1000s commented Jun 19, 2023

The git-for-windows developer has pushed a possible fix (referenced in the first issue there), so this might be resolved in the next version.

@mataha
Copy link
Contributor

mataha commented Jun 20, 2023

Can git-for-windows/git#4466 be related? Since this and that are eerily similar, I can't help but think that all these issues can be traced to a single root cause.

I believe that, just like in the link above, reverting Git to 2.36.1 will temporarily remedy the problem.

@dandavison
Copy link
Owner

I'm going to close this issue, but that's not at all meant to stop the conversation (which is fascinating!) but rather to reflect the fact that the conversation has shown that this isn't a delta bug and so it's clearer to have the issue in closed state.

@astiob
Copy link
Author

astiob commented Jun 21, 2023

The git-for-windows developer has pushed a possible fix (referenced in the first issue there), so this might be resolved in the next version.

I’m not seeing any new fixes in the two issues you linked above (4422 and 4464). Could you clarify where to look?

And everyone, do you think we may have multiple issues here? One is the hang, another is the grey background when the hang doesn’t happen, and then there’s the complete lack of colour that I originally reported. They may or may not have a common cause, but if it’s something about pipes and blocking, I wouldn’t expect it to affect colours 🤔

@th1000s
Copy link
Collaborator

th1000s commented Jun 21, 2023

I hope everyone found a workaround which works for them.

And as astiob mentioned - the strange color background is so far unexplained, maybe it is another side effect.

Issue git-for-windows/git#4422 mentions a new revert-commit (dscho/msys2-runtime@10e99fe). Also, in the other issue the reporter has posted a minimal reproducer in C++.

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

No branches or pull requests

8 participants