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

Enabling line-numbers breaks -/+ prefix stripping and staging individual hunks #13

Open
The-Compiler opened this issue Sep 9, 2020 · 15 comments

Comments

@The-Compiler
Copy link

Hey! I'm relatively new to emacs and found this package (and delta itself). Thanks for all your work, it looks amazing! 🤩

However, it looks like enabling the line-numbers feature in delta breaks various functionality in magit-delta:

  • The + and - prefixes aren't stripped anymore
  • I can't stage individual hunks anymore from the magit status buffer - when I try, I get "corrupt patch at line xy" from git

Screenshot:

image

This is on Archlinux, with emacs 27.1, Doom v2.0.9. I haven't found out what magit-delta version I'm running - Doom Emacs uses straight, so pkg-info-package-version doesn't know about magit-delta, and straight-freeze-version somehow errors out. Happy to provide more information if you tell me how to get it 🙂

@dandavison
Copy link
Owner

dandavison commented Sep 10, 2020

Yes, there's a little bit of a mess here that needs sorting out.

TL;DR: line-numbers cannot be used with magit-delta (see below). If you want to be free to use line-numbers with delta on the command line (and why not!) then (EDIT) see the suggestion below. the best workaround I can suggest currently is to add --no-gitconfig to your magit-delta-delta-args. That will make your magit-delta config independent of your main delta config. But then of course you'll have to also add to magit-delta-delta-args all the other delta command line args that you want delta to be using when it is invoked by magit-delta (e.g. colors, syntax-highlighting theme, etc).

Here's the situation:

  1. line-numbers fundamentally cannot be used with magit-delta. This is because the string that delta gives to magit must be a valid git patch (because magit assumes whatever is in its diff buffer is a valid patch and sends it to git apply to stage changes). Similarly (and perhaps more obviously), side-by-side cannot be used. Nor can any of delta's "decorations" (the boxes and lines, etc)
  2. This is the motivation for --color-only being included in the emacs variable magit-delta-delta-args. In fact, magit-delta puts that back in the args if you remove it.
  3. However, --color-only is also the recommended way to use delta with git add -p (see example gitconfig in delta README.
  4. The semantics of color-only are intended to be "just add colors to the git output, but do not make any structural changes to be made to the git output".
  5. But, people want to use line-numbers with git add -p. So currently color-only does not actively disable line-numbers if the user has set it.

@dandavison
Copy link
Owner

Actually, I think there's a better solution involving using delta "features". Try setting your magit-delta-delta-args something like this

(setq magit-delta-delta-args
  '("--24-bit-color" "always"
    "--features" "magit-delta"
    "--color-only"))

Then magit-delta will not use the line-numbers and decorations features you've declared in your gitconfig. And you can add magit-delta -specific config in a section in gitconfig like

[delta "magit-delta"]
    whitespace-error-style = green reverse  # or whatever

@wyuenho
Copy link
Contributor

wyuenho commented Jan 5, 2021

@dandavison I have this config and magit-delta-delta-args set to only '("--features" "magit-delta"), but line numbers is still showing in my magit diff and break hunk staging.

[delta]
line-numbers = true

[delta "magit-delta]
line-numbers = false

@dandavison
Copy link
Owner

dandavison commented Jan 5, 2021

Hi @wyuenho, instead of setting the line-numbers boolean directly in the main delta stanza, can you try activating it via its feature:

[delta]
    features = line-numbers

[delta "magit-delta"]
    line-numbers = false

I just tested this and it seemed to fix it. I'm not claiming this is obvious! I definitely admit that delta config when using all the various possibilities has some subtleties and complexity, but it is at least pretty configurable. There are probably improvements to be made to make it more consistent.

(There's a missing closing " in the gitconfig you posted but I think that's just a typo here not in your actual gitconfig.)

@wyuenho
Copy link
Contributor

wyuenho commented Jan 5, 2021

Does that mean I can only really have one set of config?

@dandavison
Copy link
Owner

Does that means I can only really have one set of configs anyway?

No, pretty much anything is achievable I think -- I'd be happy to try to provide a config implementing any requested behavior. The config I posted above results in line-numbers being active for delta in normal usage, but inactive when used by magit-delta. The features list can contain any number of features. There is an environment variable DELTA_FEATURES that can be used to modify active features on-the-fly (e.g. in shell-aliases etc).

For the adventurous, a feature can have its own feature list, thus yielding a tree of sets of key-value pairs, in which any conflicts are resolved according to a tree traversal order which should hopefully be intuitive but is documented in the code (here and here).

@wyuenho
Copy link
Contributor

wyuenho commented Jan 5, 2021

Your config isn't equivalent to mine. I don't want the main config to reference to magit-delta config unless I explicitly tells delta to in the command line.

I don't understand what's happening here. What's the relationship between [delta], [delta "feature"] and delta's defaults? Delta doesn't turn on line-numbers by default, but omitting it from the main section does? Does a feature section override the delta section? The answer seems to be no in both cases. What I really need is, when I tell delta to use a feature, delta will use that feature's config alone and merge with the defaults, but not any other sections.

@dandavison
Copy link
Owner

dandavison commented Jan 5, 2021

What's the relationship between [delta], [delta "feature"] and delta's defaults?

Does this help?

[delta]
    # settings here will always be applied unless the user supplies --no-gitconfig on the command line
    # These settings will override delta's defaults

[delta "my-feature"]
    # This is just a named collection of key-value pairs.
    # It has no effect at all unless the user has added "my-feature" to their "features" list.
    # They could do that either
    # (a) on the command line via `--features`, or
    # (b) in the main `[delta]` section via `features = my-feature some-other-feature`
    # (c) by setting the env var: `DELTA_FEATURES="my-feature some-other-feature"`

I don't want the main config to reference to magit-delta config unless I explicitly tells delta to in the command line.

That's right, that's how it works. The [delta "magit-delta"] settings are just sitting there doing nothing, until you pass --features=magit-delta on the command line, or otherwise add magit-delta to the list of active features.

@wyuenho
Copy link
Contributor

wyuenho commented Jan 5, 2021

The crux of the matter is, the same key in a feature section does not override the main section, because anything in the main section always applies, that's why line numbers are still showing up in my config when invoking delta --features=magit-delta. This is undesirable, either feature config should override or replace an ancestor section, there's no way to set the custom defaults in the main section now as they always override any features. That's not what defaults are suppose to do.

@dandavison
Copy link
Owner

dandavison commented Jan 5, 2021

Ah I'm sorry, I should have linked this issue: dandavison/delta#446

the same key in a feature section does not override the main section, because anything in the main section always applies,

I think that's a bug specific to line-numbers (dandavison/delta#446 ), or to delta's "builtin features". I'd like that to be fixed soon. But it's not generally true: feature config does override the main section in general.

@dandavison
Copy link
Owner

So my suggestion

[delta]
    features = line-numbers

[delta "magit-delta"]
    line-numbers = false

is a workaround for that bug. What you did definitely should have worked.

dandavison added a commit to dandavison/delta 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 to dandavison/delta 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 to dandavison/delta 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 to dandavison/delta 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
willbush added a commit to willbush/system that referenced this issue Jun 22, 2021
dandavison/magit-delta#13

using various features like line numbers etc causes issues with magit. There are
work-arounds, but it's not worth it for me because I really mostly use magit to
view diffs instead of `git diff` so I'm Ok with default delta settings.

For some reason magit needs to be refreshed when toggling the magit-delta mode
on or off. So I added a function to do both. The magit-delta diff doesn't show
the + / - signs which is sometimes useful when copy pasting the diff.
@DrWaleedAYousef
Copy link

Sorry for jumping in. I arrived here because I searched for the side-by-side option from withing emacs using magit. I found [this reply above] that confirms it does not work with magit. However, the reply says as well line-number does not work with magit. It works with me! Is this an update to a recent version?

@kaushalmodi
Copy link

@dandavison The suggestion in #13 (comment) does not work for me.

I started using delta a few days back and I love it! The I learned that you also developed magit-delta, so I tried that out, but the diff folding (M-2, M-4 bindings) in magit-status breaks for me even after I applied your suggestion above.

Diffs polluting the magit status

image

Diffs show/hide using M-2/M-4 if I remove features = line-numbers

image

Issue

As I see, if I leave my .gitconfig as shown in the second screenshot, I cannot use delta from the terminal when running git diff, etc. there.

Do you have any suggestions that work on the latest delta version?

Thanks!


Versions:

  • delta: 0.8.2
  • magit: Latest version from melpa as of today (2021-10-22)
  • emacs: Latest emacs-28 branch build as of today (GNU Emacs 28.0.60 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.22.30, cairo version 1.15.12) of 2021-10-22, built using commit 06c944cff1a8a348b9c01a92891bd12576c0896d)

@kaushalmodi
Copy link

kaushalmodi commented Oct 22, 2021

Quick update!

It looks like this config works and makes both terminal-side and magit-side delta invocation happy.

# In .gitconfig
[delta]
    # https://github.com/dandavison/magit-delta/issues/13
    # line-numbers = true    # Don't do this.. messes up diffs in magit
    #
    side-by-side = true      # Display a side-by-side diff view instead of the traditional view
  • I simply removed line-numbers = true from the config and that makes magit-delta work.
  • I still have side-by-side = true in there, and that doesn't seem to affect magit-delta!
  • But because side-by-side is enabled, the line numbers also seem to be auto-enabled when I used delta from the terminal.

Again, this would work only for folks who like to enable side-by-side.

kaushalmodi added a commit to kaushalmodi/.emacs.d that referenced this issue Oct 22, 2021
Document a workaround for the issue seen in dandavison/magit-delta#13.
yuravg pushed a commit to yuravg/.emacs.d that referenced this issue Feb 28, 2022
Document a workaround for the issue seen in dandavison/magit-delta#13.
yuravg pushed a commit to yuravg/.emacs.d that referenced this issue Mar 2, 2022
Document a workaround for the issue seen in dandavison/magit-delta#13.
willbush added a commit to willbush/system that referenced this issue Dec 28, 2022
dandavison/magit-delta#13

using various features like line numbers etc causes issues with magit. There are
work-arounds, but it's not worth it for me because I really mostly use magit to
view diffs instead of `git diff` so I'm Ok with default delta settings.

For some reason magit needs to be refreshed when toggling the magit-delta mode
on or off. So I added a function to do both. The magit-delta diff doesn't show
the + / - signs which is sometimes useful when copy pasting the diff.
lunik1 added a commit to lunik1/.doom.d that referenced this issue Jan 10, 2023
@brc
Copy link

brc commented Mar 17, 2023

Using @dandavison's suggestion worked. 👍

But I could only assign magit-delta-delta-args in Spacemacs via customize; trying to use (add-to-list) or (setq-default) in various Spacemacs places (such as (dotspacemacs/user-config)) wouldn't actually change the value. Then again, I use Spacemacs because I don't know Emacs well enough (and subsequently, I don't know Emacs well enough because I use Spacemacs)!

At one point, I had read about the relationship between (setq) and customizable variables, and what you should or shouldn't do, but I don't recall any details (...pretty much ever).

Thank you, @dandavison.

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

6 participants