Skip to content

Conversation

da-x
Copy link
Contributor

@da-x da-x commented May 6, 2020

This fixes #88.

p.arg("--no-init");
}
_ => {}
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lzybkr Would you be able to review this change from the Windows point-of-view? The code here is copied from bat. If Delta simply does not work correctly with anything other than your version of less then perhaps this version-checking doesn't make sense on Windows?

Copy link
Owner

@dandavison dandavison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much @da-x! This LGTM, since it is what bat is doing, and I've double-checked the diff between this and what I get if I just copy bat's current version of these files. I'll wait for @lzybkr's input if possible before merging since, on Windows, Delta is benefitting from his less fork (see #12 ), but bat currently is not.

Ref #116 which aims to use the bat crate and avoid vendoring bat code entirely; however currently I don't think the bat crate exposes what we need.

@dandavison dandavison merged commit 796307d into dandavison:master May 9, 2020
@dandavison
Copy link
Owner

Thanks again @da-x. This is good to have, as --no-init (-X) is really something that should be up to the user without delta mysteriously overriding it. #185 is an issue raised by a Delta user who encountered the old behavior.

I think this is the right thing to do on Windows since bat and delta users are advised to use recent versions of less on Windows which have @lzybkr's patches.

@cohml
Copy link

cohml commented Sep 25, 2024

Sorry to bring my personal problems here, but I simply cannot crack this.

When using less with delta, no matter how I configure it (e.g., via .gitconfig, via DELTA_PAGER), less does not respect --no-init. Meanwhile, --no-init works perfectly when using less with bat. I also see that when I add arguments for less via .gitconfig or DELTA_PAGER, they are respected (e.g., -N); only --no-init doesn't seem to work.

As a potentially relevant datapoint, when I do LESS= PAGER= \less some_long_file.txt, the screen is cleared upon exiting less, despite that I'm not obviously passing that in anywhere.

Anyway, can you provide any thoughts as to why --no-init might not be working with delta?

@th1000s
Copy link
Collaborator

th1000s commented Sep 30, 2024

If you are on Linux, you can check the exact way less is called:

Trace (strace) the bat and delta calls to less, writing it to a log file -o, including full syscall args and strings up to a length of 99 -v -s 99, following (process) forks -f, only looking at successful syscalls for process management -z -e process:

strace -o bat.log -v -s 99 -f -z -e process bat examplefile

vs.

git log -10 --patch | strace -o delta.log -v -s 99 -f -z -e process delta

There should be an execve(..) call to less in both logs, but ignore the one with --version in delta. It has the arguments and the env vars (do NOT post these, they might contain sensitive information), for example:

execve("/usr/bin/less", ["less", "--RAW-CONTROL-CHARS", "--quit-if-one-screen"], ["COLUMNS=165", "SECRET=01234", /* more env vars */ ]

delta will add LESSUTFCHARDEF, LESSCHARSET and LESSANSIENDCHARS env vars.

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 this pull request may close these issues.

Don't pass --no-init to less when BAT_PAGER is explicitly set

4 participants