🐛 --navigate breaks git log --oneline and empty git diff

[SimenB]

Clean repo and running git diff

Running git log --oneline gives same error.

Note that in both cases hitting Enter will make it print out the correct stuff (which makes #234 really obvious)

[dandavison]

Hi @SimenB, thanks for this report also. I am not sure yet what to do about the way less --pattern behaves, other than to recommend using --navigate only when needed -- ideas welcome!

[SimenB]

For the diff case, could you detect that it's empty and skip the pattern? For the log --oneline case, somehow detect you did no replacements (or whatever you do, I haven't looked at the source), so you know the pattern doesn't exist? The latter solution sounds more generic and would work for the former case as well?

[dandavison]

The latest release of delta allows navigate to be activated via an environment variable, e.g.:

DELTA_NAVIGATE=1 git diff

This is motivated by the fact that leaving navigate active all the time is problematic; please see the README section https://github.com/dandavison/delta#navigation-keybindings-for-large-diffs

[cben]

I've found some less tricks: https://superuser.com/a/1626732/33415
If instead of less --pattern=foo, you do less '+r/foo|$', that behaves a bit better, and at least shows the text before the annoying "Pattern not found (Enter)".
It's still not perfect, so 👍 to the env var solution.

Another thing aggravating this is the navigate regexp only matches git mode output.
It doesn't match comparing: from regular delta FILE1 FILE2, nor plain unified-diff ^--- that you see in --color-only mode.

[dandavison]

@cben thanks very much for posting this. So do you think a change to delta code is warranted? Can you help me make sure I'm getting this right? I've made a very quick PR: #526 The diff there is

-                    process.args(&["--pattern", &navigate::make_navigate_regexp(&config)]);
+                    process.args(&[format!("+r/{}", navigate::make_navigate_regexp(&config))]);

Or did you intend for delta to use the |$ part also? That's not behaving as desired for me, e.g. this does not seem to navigate on pressing n and N:

git log -p | less '+r/^commit|$'

I have some tangential questions that you might be able to help with:

• If I'm not mistaken, less remembers the last regexp used. But sometimes I want to clear this. Where is this persisted? I don't see an env var.
• Is there any way to get less to tell me what regexp has been set under --pattern or +/...?

[dandavison]

In addition to the DELTA_NAVIGATE env var, git -c ... can now be used with all delta options. I think this is an improvement in our abilities to configure delta on-the-fly. So here for example,

git -c delta.navigate=true log -p
git -c delta.navigate=false log -p

This requires delta 0.6.0.

[cben]

Ack, you don't want |$. That's a trick for not jumping to first match (which was arguably desired for that StackExchange use case) but it works by matching all lines, which breaks n/N.
I think for delta jumping is harmless as there should be some match at the start — commit or file. But if you want to suppress it, you could use the /Ctrl+K way (not sure what's the Rust escape syntax).

[cben]

• If I'm not mistaken, less remembers the last regexp used. But sometimes I want to clear this. Where is this persisted? I don't see an env var.

$HOME/.lesshst file (at least on linux).

I just checked, and both --pattern=foo and +/bar add to this history. Perhaps --no-histdups can be helpful.
Could also turn off saving with LESSHISTFILE, but that would also preclude users recalling/saving their own searches, which is bad.

• Is there any way to get less to tell me what regexp has been set under --pattern or +/...?

You might be able to configure less prompts to show it constantly. But for debugging, just press / Up arrow and you'll see it.

[cben]

Oooh, here is a weird idea (which is maybe what you were getting at): don't pass a pattern to less at all!
Instead, append it to $HOME/.lesshst, then call less. Pressing n with no active pattern uses the last one from history 😜

This has several benefits when the diff is short, including --quit-if-one-screen compatibility, and not filling the screen _until you press n or N.

Simply appending doesn't work; the format is not quite documented, need " before the pattern but there are few "sections" starting with . so it does't always belong at the end (e.g. with --save-marks there may be .mark section after .search)... The file name also varies.

We could make less append it for us, something like:

echo 'guarantee a match: 123' | less --no-histdups --save-marks $'+/123\nq'

Normally that exits immediately with no visible output, but that's an illusion — it wrote to the "alternate buffer" and switched back too fast to notice. Proof: echo $'\e[?47h' (switch to alt buffer without clearing). With --no-init, or terminal configs that don't support/use alt buffer you'll see that line plus a full screen of ~ ☹️. (This goes back to that StackExchange issue: less can't search without painting the full screen.)
And if I redirect > /dev/null, less won't actually search (it's in bypass cat-like mode), the history file is unaffected.

Don't have time to play more with this now. This is promising, but also increasingly fragile and ideally needs explicit flags for the desired search behavior in less itself.

[dandavison]

Neat! OK, this is sounding promising, let's keep playing around with this as we have time. Maybe it would help to use the env var LESSHISTFILE in order for delta to have its own transient less history file, rather than doing anything to the user's?

[dandavison]

The latest commit in #526 implements your suggestion, i.e. navigate doesn't actually change the less invocation. I'm going to be using that commit while I work on other things. (Compiling delta from source is straightforward, just need rust: [instructions])

[cben]

transient less history file — not sure, would mean that inside delta you can't recall searches from other less uses, and vice versa. 😐 Not very bad but surprising if one's goal is to configure delta in git and forget it's there.

[cben]

Found upstream issue, brain-dumped there: gwsw/less#28.

Re-discovered a simpler idea 🤣: buffer the output, count lines yourself. Instead of relying on less --quit-if-one-screen, just don't call less at all when it's short!
/me starting to peek at code...
This way PagingMode::QuitIfOneScreen would also work for any $PAGER, not just less.

[dandavison]

Yes you're probably right that this would frustrate more sophisticated less users: delta's less process should be able to recall previous less searches.

It's a detail, but seeing as the delta search regex is an implementation detail of delta --navigate, perhaps less usage in non-delta contexts should not reveal this regex in history? We could consider writing to a transient LESSHISTFILE the concatenation of their "real" less history file plus one extra appended line from delta.

As you say, this locating and writing to files on disk at start up is a bit inelegant/fragile but I think as long as whatever we do is hidden behind the navigate flag, it's OK to try to come up with something that gives a nice experience even if the implementation feels a bit unclean.

[dandavison]

This should be fixed by #526 which implements @cben's idea (#237 (comment)). So should be in the next release. If anyone's able to build on master and confirm that would be great (especially Windows). Thanks @cben!

[niedzielski]

Thanks @cben and @dandavison! Very clever! This fixes a long standing annoyance I had with git stash list.

[SimenB]

Yep, this works great now. Thanks!