Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

🚀 automatic side-by-side based on column width? #359

Open
aschmolck opened this issue Oct 20, 2020 · 15 comments
Open

🚀 automatic side-by-side based on column width? #359

aschmolck opened this issue Oct 20, 2020 · 15 comments

Comments

@aschmolck
Copy link

aschmolck commented Oct 20, 2020

Hi, many thanks for writing delta -- I've long longed for something like it and am very happy to discover it.

One question: side-by-side views are great for looking at more complicated diffs but require substantially more horizontal space, so are IMO not a good default for git diff with typical terminal widths. On the other hand it would be great to have easy access to side-by-side git diffs when doing a more thorough review with a full screen'ed terminal. Unfortunately I'm not aware to of a way to pass the -s flag through when using delta as git diff backend, or some other way to easily switch case-by-case.

Assuming something like it does not already exist, how about a mechanism to provide context dependent side-by-side in git diff based on either a user-specified minimal column width or an easy way to pass flags through something like a DELTA env var (similar to LESS)?

@dmaahs2017
Copy link

dmaahs2017 commented Oct 21, 2020

Or instead of using an env variable it could be specified in the gitconfig for the delta options. ex: side-by-side = auto or side-by-side = n_columns such that when the terminal reaches the specified width it switches to side-by-side mode

@aschmolck
Copy link
Author

aschmolck commented Oct 21, 2020

Yes, my thinking was something like a side-by-side-min-columns = 120 option in gitconfig rather than an env var, I think env var makes most sense for passing through flags directly for an individual run (or git alias).

If there is interest in principle, I'd also be happy to attempt to provide a PR for this feature.

@dandavison
Copy link
Owner

Hi @aschmolck @dmaahs2017, thanks! I agree that a DELTA env var (analogous to LESS, i.e. holding a string which behaves exactly like command-line flags) could indeed make sense. There have been some similar discussions, let's just loop in some others: @Ryuta69 @Kr1ss-XD @navarroaxel @gibfahn any opinions there?

It's not at all ideal, but what I currently do is something like

git config --global delta.side-by-side true

(as is in the README).

Also your idea of an auto-side-by-side mode sounds interesting; I hadn't thought of that. Would the ideal be to re-use the same option, rather than to introduce another one? I.e.

side-by-side = true|false|120

If there is interest in principle, I'd also be happy to attempt to provide a PR for this feature.

@aschmolck great! I'll be happy to discuss designs and review code.

@dmaahs2017
Copy link

dmaahs2017 commented Oct 21, 2020

Also your idea of an auto-side-by-side mode sounds interesting; I hadn't thought of that. Would the ideal be to re-use the same option, rather than to introduce another one? I.e.

side-by-side = true|false|120

@dandavison Yeah I think reusing this option would be the best UX. Where 120 could be any positive integer

@ghost
Copy link

ghost commented Oct 22, 2020

I think this is a good and useful idea!

side-by-side = true|false|120

This is just my opinion, but wouldn't the below example be confusing?

[delta]
    features = side-by-side
    side-by-side = 120

Would Putting side-by-side-min-columns = 120 be more intuitive...?

P.S.
I currently don't come up with any other options.

@navarroaxel
Copy link
Contributor

It's a hard decision and maybe we can't revert this latter... but I like

side-by-side = true|false|120

instead of an extra configuration variable.

But I think in the future someone will request a flag to turn off the side-by-side automatically if the compared file has a line longer than the half of the terminal, etc.

@aschmolck
Copy link
Author

@navarroaxel maybe overkill, but one option to future-proof (and self-document) it more would just be to make it more descriptive, e.g:

side-by-side = true | false | columns > 119

That's more verbose, but presumably that's not really an issue for a gitconfig line. It's a tad more annoying to parse, but again not that much.

@dandavison
Copy link
Owner

One thing we should bear in mind is that if we switch side-by-side to string type then I think the previously supported delta --side-by-side usage will be broken, because I don't think clap (the command-line arg parsing library) supports string options with no value (I'd like to be wrong about that. It's happened a few times already that I start off with a boolean and find myself wanting to expand the functionality). I still might prefer that to introducing a new option though, since delta has quite a lot of options already, and delta really emphasizes configuration via gitconfig, not command line. But some people's shell aliases etc would be broken.

@aschmolck
Copy link
Author

aschmolck commented Oct 22, 2020

@dandavison If you dislike introducing new flags, think just extending the meaning of the gitconfig option and keeping the commandline option a simple boolean switch is probably fine, especially since delta reads gitconfig even in "standalone" mode. This won't break any existing aliases and there is no super compelling reason to have an automatic toggle via commandline flag since, unlike git diff, toggling is just an -s away.

There should probably be a --no-side-by-side option though, to override the .gitconfig default (if it's true or auto).

@dandavison
Copy link
Owner

think just extending the meaning of the gitconfig option and keeping the commandline option a simple boolean switch is probably fine

Right. If I'm understanding your suggestion correctly there's precedent for this -- it's similar to what we did to allow line-numbers = false to have special meaning in gitconfig. See #292 (comment) and #296.

@dandavison
Copy link
Owner

dandavison commented Dec 27, 2020

Assuming something like it does not already exist, how about a mechanism to provide context dependent side-by-side in git diff based on either a user-specified minimal column width or an easy way to pass flags through something like a DELTA env var

@aschmolck Delta now supports an env var named DELTA_FEATURES, which can be used to dynamically alter the set of active features (a "feature" in delta terminology is either a named set of delta options using the [delta "my-feature"] heading syntax in ~/gitconfig, or one of the built-in "features" such as side-by-side). So we have one way of implementing automatic side-by-side switching. Here's a proof-of-principle in zsh (bash would be similar, using PROMPT_COMMAND):

__dan_preexec_function () {
    local columns=$(tput cols)
    if [ $columns -ge 140 ]; then
        export DELTA_FEATURES="side-by-side"
    else
        export DELTA_FEATURES=""
    fi
}
typeset -ag preexec_functions;
preexec_functions=( __dan_preexec_function ${preexec_functions[@]} )

This is similar to wfxr/forgit#121 (comment) and may be relevant to #368; the shell code could also depend on FZF_PREVIEW_COLUMNS cc @mrswats @neerajbadlani @ulwlu

I do wonder whether we would also want a DELTA_ARGS env var to literally add command line options. DELTA_FEATURES allows activating arbitrary groups of options by name and so is quite powerful, but it's not great for setting a numerical value like --width dynamically.

@dandavison
Copy link
Owner

I do wonder whether we would also want a DELTA_ARGS env var to literally add command line options.

There is GIT_PAGER however, which can be used to dynamically alter the delta command line when delta is invoked by git, so here's a second way of doing automatic side-by-side switching:

__dan_preexec_function () {
    local columns=$(tput cols)
    if [ $columns -ge 140 ]; then
        export GIT_PAGER="delta --side-by-side"
    else
        export GIT_PAGER="delta"
    fi
}
typeset -ag preexec_functions;
preexec_functions=( __dan_preexec_function ${preexec_functions[@]} )

@zeorin
Copy link

zeorin commented Mar 15, 2021

Here's a Fish shell version that listens to SIGWINCH:

# Determine whether to use side-by-side mode for delta
function delta_sidebyside --on-signal WINCH
	if test "$COLUMNS" -ge 120; and ! contains side-by-side "$DELTA_FEATURES"
		set --global --export --append DELTA_FEATURES side-by-side
	else if test "$COLUMNS" -lt 120; and contains side-by-side "$DELTA_FEATURES"
		set --erase DELTA_FEATURES[(contains --index side-by-side "$DELTA_FEATURES")]
	end
end
delta_sidebyside

@cohml
Copy link

cohml commented Jan 29, 2024

Coming to this thread from the distant future...

I see almost universal enthusiasm for this proposal in the thread, plus at least one comment volunteering to work on it. Yet three years have passed and the issue remains open, nor does any PR link to it.

Was this feature ever added and we just forgot to close the issue? Or was there some principled reason it never got implemented?

@dandavison
Copy link
Owner

dandavison commented Jan 29, 2024

Hi @cohml, it's that no-one worked on it.

FWIW, I recently improved my shell tooling for toggling delta settings. This is what I've settled on currently:

delta-toggle() {
    eval "export DELTA_FEATURES=$(-delta-features-toggle $1 | tee /dev/stderr)"
}

where -delta-features-toggle is https://github.com/dandavison/tools/blob/main/python/-delta-features-toggle.

So

delta-toggle    # shows current features
delta-toggle s  # toggles side-by-side
delta-toggle l  # toggles line-numbers

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants