dandavison / delta

A syntax-highlighting pager for git, diff, grep, and blame output
https://dandavison.github.io/delta/
MIT License
23.01k stars 382 forks source link

🐛 no output from git diff #360

Closed brandly closed 3 years ago

brandly commented 4 years ago

hello! thanks for making this. i look forward to using it more.

i installed this via brew install git-delta, and i've added these lines to my ~/.gitconfig.

[core]
    pager = delta

[delta]
    plus-color = "#012800"
    minus-color = "#340001"
    syntax-theme = Monokai Extended
    side-by-side = false
    line-numbers = true

[interactive]
    diffFilter = delta --color-only

mouse scrolling didn't work, so i added export BAT_PAGER="less -RF" to my .profile as mentioned in the readme.

for the most part, things look great. git show looks great.

i made some quick file changes to try git diff. i can walk through the changes with git add -p, but git diff produces no output at all!

$ git diff
$ echo $?
0

i'm on macos 10.15.7 using Terminal, and i have oh my zsh installed. i can provide other details, but i'm unsure how to dig into this!

dandavison commented 4 years ago

Hi @brandly, is it possible that you staged all your changes? In that case you'll need to do git diff --cached. Also, as a debugging technique, what happens when you do git diff | cat? (That won't use delta, so if you see no output, it's not delta's fault). If git diff | cat is showing a diff, but git diff is not, then could you please post the diff here? (if you can share the content.)

brandly commented 4 years ago

i'm not sure what was wrong with that terminal. i opened a new one, and i'm now everything's working perfectly!

i installed it from that shell, was already in a repo's directory, and made some changes to view them. the changes weren't staged. there's nothing sensitive here, so i'll share in case it helps.

$ git diff | cat
diff --git a/src/RowCollection.tsx b/src/RowCollection.tsx
index 530dd3a..901e936 100644
--- a/src/RowCollection.tsx
+++ b/src/RowCollection.tsx
@@ -385,7 +385,7 @@ export const SourceChart = ({
       )
     })
     return (
-      <ChartRow height={height} axisMargin={0} key={display}>
+      <Hello height={height} axisMargin={0} key={display}>
         {getSmallYAxis(display, [])}
         <Charts>
           <EventChart
$ git diff

i quickly assumed the installation was broken or something, but viewing these same changes in another terminal instance worked fine. sorry to bother you!

brandly commented 3 years ago

this is happening again! same kinda thing. i opened another terminal instance and it went away.

$ delta --version
delta 0.4.4
$ which delta
/usr/local/bin/delta
$ git diff
$ git diff | cat
diff --git a/package-lock.json b/package-lock.json
index 506a119..1d1bdf5 100644
--- a/package-lock.json
+++ b/package-lock.json
@@ -1348,9 +1348,9 @@
       }
     },
     "@types/node": {
-      "version": "14.11.10",
-      "resolved": "https://registry.npmjs.org/@types/node/-/node-14.11.10.tgz",
-      "integrity": "sha512-yV1nWZPlMFpoXyoknm4S56y2nlTAuFYaJuQtYRAOU7xA/FJ9RY0Xm7QOkaYMMmr8ESdHIuUb6oQgR/0+2NqlyA==",
+      "version": "14.14.2",
+      "resolved": "https://registry.npmjs.org/@types/node/-/node-14.14.2.tgz",
+      "integrity": "sha512-jeYJU2kl7hL9U5xuI/BhKPZ4vqGM/OmK6whiFAXVhlstzZhVamWhDSmHyGLIp+RVyuF9/d0dqr2P85aFj4BvJg==",
       "dev": true
     },
     "@types/prop-types": {
diff --git a/package.json b/package.json
index ff07812..57edba7 100644
--- a/package.json
+++ b/package.json
@@ -20,7 +20,7 @@
     "use-debounce": "5.0.1"
   },
   "devDependencies": {
-    "@types/node": "14.11.10",
+    "@types/node": "14.14.2",
     "@types/react": "16.9.53",
     "@types/react-beautiful-dnd": "13.0.0",
     "@types/react-dom": "16.9.8",

is there more i can do to diagnose? is delta stateful at all, such that the issue disappears in a fresh terminal? or is the issue outside of delta?

dandavison commented 3 years ago

Hi @brandly, what happens if you do git diff | cat | delta? I assume you then see the diff again? Delta doesn't seem to have any problem showing your diff. I saved it to file, and then piped it to delta:

image

What exactly do you have in your core.pager section of ~/.gitconfig? Here's how delta works: it takes text on standard input, it reads settings, it starts a child less process, and it sends its output to that child less process. It's not stateful. When you do git diff, with delta set in core.pager in gitconfig, then git starts a delta process automatically (assuming the core.pager value identifies the delta executable correctly) and passes git's output to delta.

I'm trying to think what problem's affecting you but I don't think it's a bug in delta. Try experimenting with different values in the core.pager setting of gitconfig, for example,

[core]
    pager = cat

or

[core]
    pager = bash -c 'echo hello world!'
brandly commented 3 years ago

I assume you then see the diff again?

i don't! very strange:

$ git diff
$ git diff | cat
diff --git a/package.json b/package.json
index 57edba7..1293beb 100644
--- a/package.json
+++ b/package.json
@@ -1,8 +1,6 @@
 {
   "scripts": {
     "start": "parcel src/index.html",
-    "deploy": "parcel build src/index.html --public-url ./ && gh-pages --dist dist/",
-    "cluster:start": "parcel src/cluster.html",
     "cluster:build": "parcel build src/cluster.html --public-url ./",
     "typecheck": "tsc -p ./ --noEmit --skipLibCheck",
     "test": "npm run typecheck"
$ git diff | cat | delta
$ delta
Usage: delta minus_file plus_file
$ echo $BAT_PAGER
less -RF

here's the end of my ~/.gitconfig

[core]
    pager = delta

[delta]
    plus-color = "#012800"
    minus-color = "#340001"
    syntax-theme = Monokai Extended
    side-by-side = false
    line-numbers = true

[interactive]
    diffFilter = delta --color-only

changing it to pager = cat, i get a simple colored diff. with pager = bash -c 'echo hello world!', i get the expected "hello world". with pager = delta, i get nothing!

if you think the problem is outside of delta, i will leave you alone! like i said, it happens in this terminal instance but not in a fresh one. it seems unrelated to the diff, since delta has no problem handling the original diff, and the problem occurs with a simple line removal.

i like to think i'm fairly familiar with git, but these pager details are new to me. i appreciate the explanation! maybe you have further insights, since these results seem surprising.

chrishiste commented 3 years ago

I had a similar problem. Git diff worked only the 1st time (for each new terminal). Setting DELTA_NAVIGATE fixes the error.

DELTA_NAVIGATE=1 git diff

This might be related to the fact that I use https://github.com/sharkdp/bat but I don't know much about pager either.

dandavison commented 3 years ago

@brandly, I'm sorry, I missed your last post. This is strange! We must be able to get to the bottom of it. I think, like @chrishiste suggests, it must be something to do with less.

As a sanity check, can you confirm that this prints hello to the screen?

echo hello | delta --no-gitconfig | cat

That did not use less.

Now, what about if you repeat that but without the final pipe to cat? Delta will see it's output is not being piped and it will send it to the pager (less)

echo hello | delta --no-gitconfig
dandavison commented 3 years ago

@chrishiste thanks, you're right, there is something stateful: less remembers the last regex that was used for searching, and so this does make delta stateful in a way when you are using navigate.

@chrishiste and @brandly can you make sure that the DELTA_NAVIGATE env var is not set to anything, e.g. checking that with

env | grep DELTA

and make sure delta is not using navigate = true or --navigate?

As long as navigate is not being used, I don't think there is anything else stateful going on that would have "memory".

dandavison commented 3 years ago

To deactivate the DELTA_NAVIGATE env var you have to either use the shell command unset, or start a new shell process in which it is not set (see the README section)

chrishiste commented 3 years ago

All those command print nothing. I don't have navigate turned on globally.

dandavison commented 3 years ago

All those command print nothing

😧 Hmm! What about this

echo hello | delta --no-gitconfig | /bin/cat

(Also what's your OS and what is which delta and delta --version?)

chrishiste commented 3 years ago

This one works.

I'm running

brandly commented 3 years ago

catching up on this convo!

$ echo hello | delta --no-gitconfig | cat
hello
$ echo hello | delta --no-gitconfig
$ env | grep DELTA

same setup:

dandavison commented 3 years ago

So in both your cases it looks like something's going wrong with your pager (i.e. less). Perhaps related to it remembering a previous regex search, or less's -p / --pattern flag.

@chrishiste I think echo hello | delta --no-gitconfig | cat is not working for you because you have cat aliased to bat, and bat is using less. I would kind of expect that bat (and therefore cat) just isn't working for you at all, independent of delta.

In any case, could you take a look (and post) the values of the environment variables BAT_PAGER, DELTA_PAGER and LESS?

chrishiste commented 3 years ago

@dandavison That's correct I do have alias cat=bat but bat is working on my system.

echo hello | delta --no-gitconfig | cat this works if I remove the alias.

And here's the output

❯ echo $BAT_PAGER
less -RF
❯ echo $DELTA_PAGER

❯ echo $LESS
-R

It's weird $DELTA_PAGER is empty. I do have the following in my global git config.

[core]
    pager = delta
dandavison commented 3 years ago

It's weird $DELTA_PAGER is empty. I do have the following in my global git config.

Actually, there's no reason for the DELTA_PAGER environment variable to be set unless you have set it yourself. It's not necessary, it's just a way to specify what pager delta should use, but delta will default to less with some sensible arguments.

bat is working on my system.

Hm, so to confirm, are you saying this prints out "hello" (with bat's decorations)

echo hello | bat

and this prints out "hello"

echo hello | delta --no-gitconfig | /bin/cat

but this prints out nothing (or just a newline character)

echo hello | delta --no-gitconfig | bat

?

chrishiste commented 3 years ago

@dandavison The pipe to bat doesn't work indeed. You must be right, I have a problem with bat.

❯ echo hello | bat (newline)

❯ echo hello | delta --no-gitconfig | /bin/cat (newline) hello

❯ echo hello | delta --no-gitconfig | bat (newline)

I'm going to look into why bat is not working. If you have an idea let me know. Sorry for the confusion.

dandavison commented 3 years ago

No problem! I'm still guessing it's something to do with less. Note that both bat and delta work in a similar way: if they see that the output is being piped, they will just send their output into the pipe. But otherwise (if it's going to screen) they will start a less process and send their output to that process.

So. Bear with me...I know there have been a lot of piping experiments... what I'm guessing is that the following will work (because it prevents bat from using less)

echo hello | bat | /bin/cat

If I'm right about that, then I'm thinking the problem is one of the following:

  1. Something about the options you're passing to less. See BAT_PAGER, PAGER and LESS env vars. Try unsetting them all:
echo hello | BAT_PAGER= PAGER= LESS= bat

(If that looks weird to you, it's, yes shell languages are a bit weird, but doing X= sets env var X to empty string, and writing it like that before the command sets the env var just for the duration of that child process.)

  1. Your less version is bad (you're on macos right? Maybe install the version from homebrew. E.g. you can see I have both on my MacOS system with homebrew taking precedence
$ type -a less
less is an alias for less -S
less is /usr/local/bin/less
less is /usr/bin/less
  1. Which reminds me: check you don't have an alias for less. You could install from homebrew and do this to ensure you are not getting an alias:
echo hello | BAT_PAGER=/usr/local/bin/less bat

OK, sorry for the random dump, hopefully some of those debugging ideas help.

dandavison commented 3 years ago

The homebrew version is newer than the one that ships with the mac:

$ /usr/local/bin/less --version
less 551 (PCRE regular expressions)

$ /usr/bin/less --version 
less 487 (POSIX regular expressions)
chrishiste commented 3 years ago

@dandavison Nice job it works now. Thanks a bunch. I've installed less from brew. Similar to the git that ships to macOS it's quite antique. I didn't know.

Resetting all envs also did the trick.

Maybe we should update the docs to tell people to install the latest less from brew on macOS.

I found this on the bat docs

If you want to enable mouse-wheel scrolling on older versions of less, you can pass just -R (as in the example above, this will disable the quit-if-one-screen feature). For less 530 or newer, it should work out of the box.

I do have mouse-wheel scrolling enabled on iTerm2 and I was using less 487.

Do you recommend setting BAT_PAGER=/usr/local/bin/less?

chrishiste commented 3 years ago

@brandly I think the same solution should work for you too. Can you give it a try?

brandly commented 3 years ago

@chrishiste updating less fixed it! great work tracking this down.

Maybe we should update the docs to tell people to install the latest less from brew on macOS.

this would be great. should it be listed as a dependency on homebrew?

chrishiste commented 3 years ago

@brandly Awesome. I will leave that decision to the maintainers :)

dandavison commented 3 years ago

OK, awesome. Yes, I agree, I'll make the README clearer about the importance of less versions, and in particular that on MacOS one should use the version from Homebrew. The Windows section has a bit about this but it's not really mentioned elsewhere.

Do you recommend setting BAT_PAGER=/usr/local/bin/less?

I think that might vary by personal style. Personally, I prefer to make sure that my shell $PATH is giving precedence to /usr/local/bin/ (yours probably already is) and rely on that rather than hard-coding absolute paths in various places in my config files.

chrishiste commented 3 years ago

@dandavison Perfect. Thanks for your help, and all the tips!