Closed tarsius closed 4 years ago
could you be more specific about "running the same in emacs"? i can't reproduce this with a simple M-!
, and i don't use magit. if you have a smaller snippet of elisp i can run, that would be ideal
(0 exit code is intentional, and similar to the behavior of git add
with no arguments)
What Magit does boils down to:
(start-file-process "*git-absorb*" "*git-absorb*" "git" "absorb" "-b" "HEAD~10")
that repros, thanks. this is clearly some kind of bug in how we're using slog... maybe even an upstream bug with the slog term drain. my first instinct is it was something to do with isatty detection (since the logger uses that to determine whether the output should be colored), but it wasn't an issue when redirecting output to a file, so it's probably something more subtle. i'll take a look over the weekend
As @marsam noted over at https://github.com/magit/magit/issues/3723#issuecomment-632829263, this can be worked around:
(let ((process-environment process-environment))
(setenv "TERM" nil) ; value before unsetting was "dumb"
(start-file-process "*git-absorb*" "*git-absorb*"
"git" "absorb" "-b" "HEAD~10"))
hrm, it seems that what's happening here is:
TERM
when executing the processTERM=dumb
and returns a fancy terminal object based on /usr/share/terminfo/d/dumbwe could ignore_res the drain instead of fusing it, but that wouldn't really be an improvement... the logs would simply be silenced with no explanation. we can also forcibly disable color (which avoids the codepaths in the term crate that are returning the error), but that's a crummy ux when you're actually on an interactive terminal. i'll see if i can find the place in slog-term that's returning the error and submit an upstream patch for it to fall back to non-fancy-terminal output instead.
@tarsius give it another try with 0.6.1? shouldn't need the (setenv "TERM" nil)
hack now
I have tried with 0.6.2
and it did not work for me so far.
But the reason for that might simply be that I don't know how to handle cargo
properly. I just ran cargo install git-absorb
again and that told me that it was:
...
Compiling slog-term v2.5.0
Compiling failure v0.1.8
Compiling git2 v0.9.2
Compiling git-absorb v0.6.2
Finished release [optimized] target(s) in 55.77s
Replacing /home/jonas/.cargo/bin/git-absorb
Replaced package `git-absorb v0.6.2` with `git-absorb v0.6.2` (executable `git-absorb`)
I am confused about the first quoted line. Shouldn't that somehow mention 1a1f6ecaec82c6ade58806e6b315a1e888d9c2b4 due to what you did in 3e14d55a8319ffe828f46d667d37a78af6ca194d? I.e. I am not sure I am actually using the slog-term
"version" that contains the fix.
huh... this is on the fringes of my understanding as well, but it looks like cargo install ignores patches when installing from crates.io. try cloning this repo and running cargo install --path .
from inside it? that seems to respect the patch directive. alternatively, if you're on osx, you could wait for someone to bump the sha of the homebrew formula, which will probably also pick up the patch. sorry for the inconvenience either way
I can confirm that it works.
I am running
git absorb
from Emacs, i.e. without a terminal.This works, but not if it needs to report an error such as (when running in
xterm
):(By the way the exit-code was 0 but should be non-zero to indicate failure. But that's a second issue.)
Running the same in Emacs results in:
It appears that it tries to perform an "operation [that is] not supported by the terminal" to show the warning at the very top. I don't know what that operation is, but am guessing that it is not actually necessary. Could this be changed to just use stdout or errout instead of the "unsupported terminal operation"?