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

🐛 Delta does not respect git -c #493

Closed
wyuenho opened this issue Jan 7, 2021 · 9 comments · Fixed by #495
Closed

🐛 Delta does not respect git -c #493

wyuenho opened this issue Jan 7, 2021 · 9 comments · Fixed by #495

Comments

@wyuenho
Copy link

wyuenho commented Jan 7, 2021

If I have the following in git's global .gitconfig:

[delta]
side-by-side = true

When I issue the following git command:

$ git -c delta.side-by-side=false diff ./somefile

I still get a side by side view. git -c should always override the config file.

@dandavison
Copy link
Owner

dandavison commented Jan 7, 2021

Hi @wyuenho, agreed, I'd like to make this work, let's figure out how. So, I had personally thought that it wasn't really possible, but I think I'm wrong (see below). First, here's how I thought it worked:

  1. A git process is created. It reads gitconfig from disk, and mutates the config in-memory according to the values passed on the command line by -c.
  2. The git process looks as core.pager and creates a child process using that command. So in our case, the child process is delta.
  3. On startup, delta reads gitconfig from disk using libgit2. So, you can see why I thought that it was not possible: delta is reading gitconfig from disk, whereas the -c modifications have been made only in the memory of the parent process.

HOWEVER...

diff-so-fancy works in the same way as delta (set as core.pager and d-s-f also uses gitconfig for some of its own settings such as diff-so-fancy.changeHunkIndicators. And what you're requesting does work with d-s-f. It is written in perl and reads gitconfig from disk like this:

		my $cmd = "git config --list";
		my @out = `$cmd`;

So the question that I do not yet know the answer to is: how is it that the child process learns of config modifications made by git -c? Do you know the answer?

Here's an experiment that doesn't involve d-s-f:

[core]
	pager = git --no-pager config --list > /tmp/seen-by-pager.gitconfig

If I run git show and then git -c user.name=xxx show, I see that the difference between the two outputs written to /tmp/seen-by-pager.gitconfig is that user.name=xxx is in the second. So the child git process learns of the modification to gitconfig that is made according to the command line arguments of the parent process, despite the fact that the child git process is reading from disk.

So, how does this work? I think I need to know that in order to fix this. I checked env and I didn't see git passing any magic env vars to the child process. Is there a secret file used that I don't know about? Or am I being dumb and missing something obvious?

@dandavison
Copy link
Owner

dandavison commented Jan 8, 2021

OK, a bit more experimenting showed the answer. It seemed like the only sane way for the parent git process to inform its grandchild git process (child of diff-so-fancy) about the -c config mutations would be via an env var. And indeed, calling env in the diff-so-fancy / delta code and diffing the output with and without -c reveals that an env var named GIT_CONFIG_PARAMETERS is used to do this. While d-s-f uses git config --list to read config, delta uses libgit2. Unfortunately it's an open issue in libgit2 to honor this env var: libgit2/libgit2#3854.

So the solution to this ticket would be either

  1. Use git config --list instead of libgit2, but I don't want to do that! (Much too fragile trying to find the user's git; delta doesn't even require git to be present and can be used without it.)
  2. Parse the value of the env var ourselves and apply the mutations to the config from libgit2.
  3. Wait for it to be done in libgit2, which would be the cleanest solution by some way, but doesn't look like anyone's working on that ticket from 3 years ago.

@wyuenho
Copy link
Author

wyuenho commented Jan 8, 2021

  1. Sounds easy enough

dandavison added a commit that referenced this issue Jan 8, 2021
When git is invoked as `git -c aaa.bbb=ccc -c ddd.eee=fff` then git
sets the env var GIT_CONFIG_PARAMETERS containing the changed config
entries, so that child processes can honor them.

libgit2 doesn't yet honor the env var: see
libgit2/libgit2#3854.

Fixes #493
Fixes #307
Ref dandavison/magit-delta#13
dandavison added a commit that referenced this issue Jan 8, 2021
When git is invoked as `git -c aaa.bbb=ccc -c ddd.eee=fff` then git
sets the env var GIT_CONFIG_PARAMETERS containing the changed config
entries, so that child processes can honor them.

libgit2 doesn't yet honor the env var: see
libgit2/libgit2#3854.

Fixes #493
Fixes #307
Ref dandavison/magit-delta#13
dandavison added a commit that referenced this issue Jan 8, 2021
When git is invoked as `git -c aaa.bbb=ccc -c ddd.eee=fff` then git
sets the env var GIT_CONFIG_PARAMETERS containing the changed config
entries, so that child processes can honor them.

libgit2 doesn't yet honor the env var: see
libgit2/libgit2#3854.

Fixes #493
Fixes #307
Ref dandavison/magit-delta#13
dandavison added a commit that referenced this issue Jan 8, 2021
When git is invoked as `git -c aaa.bbb=ccc -c ddd.eee=fff` then git
sets the env var GIT_CONFIG_PARAMETERS containing the changed config
entries, so that child processes can honor them.

libgit2 doesn't yet honor the env var: see
libgit2/libgit2#3854.

Fixes #493
Fixes #307
Ref dandavison/magit-delta#13
@dandavison
Copy link
Owner

Thanks @wyuenho, I think this will be a big improvement to configuration on the fly. Now all we need is shell completion over delta options :)

@kbd
Copy link

kbd commented Apr 16, 2021

Is this in version 0.7.1? With side-by-side in features in main delta settings, trying to do:

git -c delta.side-by-side=false diff @~

and still getting a side-by-side view.

Also, question. With config like:

[delta]
	features = side-by-side line-numbers decorations

[delta "inline"]
	side-by-side = false

Is the intent that you can say git -c delta.inline=true diff (and retain line-numbers etc.)?

@dandavison
Copy link
Owner

dandavison commented Apr 16, 2021

Hi @kbd, you're right that this is a bit confusing. Essentially there are two to activate a "built-in feature" such as side-by-side (or navigate, diff-so-fancy, hyperlinks, etc):

  1. Inclusion in features array
  2. As a boolean-valued option

Furthermore, trying to disable something that is activated by inclusion in the features array doesn't always go as expected and I think some more work could be done there. But if you have side-by-side = true then git -c delta.side-by-side=false will deactivate it.

Is the intent that you can say git -c delta.inline=true diff (and retain line-numbers etc.)?

Not delta.inline=true, but rather git -c delta.features=inline. That works on your example. However...annoyingly it doesn't work here:

[delta]
    features = line-numbers decorations
    side-by-side = true

[delta "inline"]
    side-by-side = false
    plus-style = red red

In summary, there is more work to be done on straightening out the semantics and fixing bugs, but it is usually possible to achieve whatever you want with a little experimentation. I know this has been noted a few times. Let's use #446 for this; that is closely related.

@jacobmischka
Copy link

jacobmischka commented May 1, 2021

But if you have side-by-side = true then git -c delta.side-by-side=false will deactivate it.

This does not seem to be the case in delta 0.7.1.

$ delta --version
delta 0.7.1

$ tail ~/.gitconfig
[delta]
	features = decorations
	side-by-side = true
	whitespace-error-style = 22 reverse
[delta "decorations"]
	commit-decoration-style = bold yellow box ul
	file-style = bold yellow ul
	file-decoration-style = none

$ git -c delta.side-by-side=false diff

Still shows side-by-side view.

image

I can't get a version of the disable feature trick to work either.

$ git -c core.pager="delta --features=no-side-by-side" diff .gitconfig

.gitconfig
───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────

────┐
64: │
────┘
│ 64 │    commit-decoration-style = bold yellow box ul               │ 64 │    commit-decoration-style = bold yellow box ul
│ 65 │    file-style = bold yellow ul                                │ 65 │    file-style = bold yellow ul
│ 66 │    file-decoration-style = none                               │ 66 │    file-decoration-style = none
│    │                                                               │ 67 │[delta "no-side-by-side"]
│    │                                                               │ 68 │    side-by-side = false

@dandavison
Copy link
Owner

Hi @jacobmischka, you're right, and I noticed this also when I upgraded my git version. It's due to a change introduced in git v2.31.0 (am I right that you have git >= 2.31.0?) I've updated delta to accomodate the upstream changes in git in #573, which will be in the next release. If you're able to build from master then it has that fix.

@jacobmischka
Copy link

Great, thank you!

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 a pull request may close this issue.

4 participants