less icon indicating copy to clipboard operation
less copied to clipboard

--pattern overrides -F and tildes up the screen

Open ScoobyDone opened this issue 6 years ago • 6 comments

Using: v530 on cygwin v458 on centos/sles (yay, enterprise distros :( )

.gitconfig snippage pager.core = less -IqRFX pager.diff = less -IqRFX --pattern='^diff --git'

With a small git diff, --pattern overrides the -F so the screen is filled with tildes (or newlines if -~ is used) and the top line(s) scroll(s) off the screen. Did some fiddling with -a and prepending @, ^K to the pattern but no change in the unintended behaviour.

Hack that fixes it in the diff, but it would be better to fix it in less itself, especially since this hack breaks less from working with a "larger than a screenfull" change, which is the use case for less in the first place...

Output with pager.diff UNset:

total 212,992
drwxr-xr-x+ 1 Scooby None      0 Oct 14 15:47 ./
drwxr-xr-x+ 1 Scooby None      0 Oct 14 15:46 ../
drwxr-xr-x+ 1 Scooby None      0 Aug 13  2016 hooks/
drwxr-xr-x+ 1 Scooby None      0 Sep  6 18:31 info/
drwxr-xr-x+ 1 Scooby None      0 Sep  6 18:31 logs/
drwxr-xr-x+ 1 Scooby None      0 Oct 14 15:27 objects/
drwxr-xr-x+ 1 Scooby None      0 Oct 14 13:08 refs/
drwxr-xr-x+ 1 Scooby None      0 Aug 17  2016 rr-cache/
-rw-rw-r--  1 Scooby None     31 Aug  1  2016 ADD_EDIT.patch
-rw-rw-r--  1 Scooby None  8,078 Aug  1  2016 ADD_EDIT.patch~
-rw-r--r--  1 Scooby None  1,094 Oct 14 13:16 addp-hunk-edit.diff~
-rw-rw-r--  1 Scooby None     36 Oct 14 15:27 COMMIT_EDITMSG
-rw-r--r--  1 Scooby None    695 Oct 14 13:07 COMMIT_EDITMSG~
-rw-rw-r--  1 Scooby None    171 Oct 14 14:55 config
-rw-r--r--  1 Scooby None    200 Oct 14 14:54 config~
-rw-rw-r--  1 Scooby None     73 Jul 26  2016 description
-rw-rw-r--  1 Scooby None      0 Sep  6 18:17 FETCH_HEAD
-rw-rw-r--  1 Scooby None 22,359 Sep  4 14:39 gitk.cache
-rw-r--r--  1 Scooby None     23 Oct 14 13:07 HEAD
-rw-r--r--  1 Scooby None 70,358 Oct 14 15:27 index
-rw-r--r--  1 Scooby None      0 Oct 14 15:27 MERGE_RR
-rw-r--r--  1 Scooby None     41 Oct 14 13:15 ORIG_HEAD
-rw-rw-r--  1 Scooby None    105 Sep  6 18:31 packed-refs
Scooby@M6800:~/  master
$ gd .gitconfig
diff --git i/.gitconfig w/.gitconfig
index defb44e..3713888 100644
--- i/.gitconfig
+++ w/.gitconfig
@@ -12,6 +12,11 @@
     mergeoptions = --no-ff
       sshCommand = ssh -o VisualHostKey=no

+[pager]
+    # hack to fix less filling and scrolling the page with tildes when --pattern is specified
+#    diff = less -IqRFX --pattern='^diff --git' | sed 's/^~\\n//g'
+#     diff = less -IqRFX --pattern='^diff --git'
+
 [branch]
  autosetupmerge = true

Scooby@M6800:~/  master
$

Output with pager.diff set:

index defb44e..f6f4d0c 100644
--- i/.gitconfig
+++ w/.gitconfig
@@ -12,6 +12,11 @@
     mergeoptions = --no-ff
       sshCommand = ssh -o VisualHostKey=no

+[pager]
+#     hack to fix less filling and scrolling the page with tildes when --pattern is specified
+#    diff = less -IqRFX --pattern='^diff --git' | sed 's/^~\\n//g'
+     diff = less -IqRFX --pattern='^diff --git'
+
 [branch]
  autosetupmerge = true

~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
~
Scooby@M6800:~/  master
$

ScoobyDone avatar Oct 14 '19 19:10 ScoobyDone

Is it possible to create a test case that exhibits the problem that does not involve git? I've tried reproducing this but can't quite follow what changes are being made to gitconfig, nor do I have the same files you have, so my git output is different.

gwsw avatar Oct 20 '19 17:10 gwsw

I believe https://superuser.com/questions/951405/less-with-f-option-and-pattern-matching/1626732#1626732 presents the same scenario but without git. Minimal repro:

seq 10 | less --quit-if-one-screen  # behaves nicely
seq 10 | less --quit-if-one-screen +/8  # not so nice

I'd say --quit-if-one-screen has 3 nice qualities, all of them approximating | less not even being there (when short):

  1. exit without need to press 'q'
  2. the output remains visible, not hidden on alternate buffer
  3. the output takes just the lines it took, not a full screen.

--no-init shares benefit (2), and starts in (3) state. If you press q immediately, it leaves just the input lines; if you scroll or search it "upgrades" to filling the screen.

Main issue is that --pattern=... gives up on (3). AFAICT any use of search (via +/... or interactive /) does that, pads the screen with ~ lines even when output is short.

Minor issue about (2): --pattern or +/... also skips to first match; if less immediately quits, the lines before that are not even available in scrollback buffer.

  • https://github.com/dandavison/delta/issues/237 has a more specialized use case. delta --navigate calls less with a useful pattern on command line, but search is not the main goal — it's just there for users to be able to press n/N for "logical" navigation. I think this is also ScoobyDone's use case.

@gwsw WDYT of following ideas: (getting out-of-topic for this specific issue, but trying to map the landscape...)

  • --no-init could try to delete trailing ~ lines on exit? The previous command is already scrolled out, but at least next command would start right after file's end.
    • Less could always try to delete trailing ~ lines on exit? (can't really know whether ti/te used alternate screen).
  • Kludge: --quit-if-one-screen, when it does fit, could ignore --pattern, + etc. Just behave like cat when it's short! Dubious, breaks compat, but could get a new option e.g. --noop-if-one-screen?
  • --quit-if-one-screen --pattern=... should avoid filling the screen on search when the output fits. Just highlight matches and exit.
  • Generally, --no-init should avoid filling the screen when the output fits. Do everything less does within the lines the output took. (probably nasty to implement. EDIT: or impossible? man termcap/terminfo say ti is mandatory before using cursor motion sequences.)
  • A new flag to set "current pattern" but not activate search until user presses n/N?

Hmm, the --noop-if-one-screen is feasible as an external wrapper: a program that buffers input, counts lines, then either just dumps it or calls less. Pro: would work today with any less version. I suspect people already invented this, more than once :-) But WDYT of making it builtin?

cben avatar Feb 17 '21 17:02 cben

I'm not sure there's anything that can possibly be done here. When a --pattern option is given, less wants to display the screen with the search result positioned at the target line. So we can't start with a less-than-full-page screen (especially if -G is set).

gwsw avatar Jan 20 '23 00:01 gwsw

Just for the record, delta solved the use case of "don't really need search on start, just want to pre-load a pattern to be used if user presses n" by not passing --pattern at all, rather adding it to $HOME/.lesshst. OP's use case for git can probably use similar approach.


[edit: all here tested with less-590-5.fc37.x86_64 Fedora linux package.]

It occurs to me now that there is also a time dimension to consider when dealing with a slow producer on stdin. Here is a convenient reproducer:

time seq 1 100000000 | grep --line-buffered 1234567 | less --QUIT-AT-EOF

(emits 20 lines over ~20sec on my machine; raise that to say 500000000 to fill a screen and take proportionally longer)

less has always supported that nicely, showing incoming input as-it's-arriving. :clap:

  • But searching e.g. | less --pattern=77 breaks that, it waits until EOF. :hourglass_flowing_sand:
  • | less --quit-if-one-screen also breaks that: it displays nothing until EOF OR until it filled one full screen. (understandable, it needs to know whether to emit ti) :grey_question:
  • Aha, | less --no-init --quit-if-one-screen is better — shows data as-it's-coming, upgrades to interactive fullscreen if > screen (with usual consequence of not using alternate screen — displayed content remains after exit).
  • Alas, | less --no-init --quit-if-one-screen --pattern=77 still waits until EOF.

  • BTW, one limitation of | less is AFAICT it's non-interactive until input is done? You can start typing commands e.g. /77Enter but I guess they just stay in tty's buffer, won't be visible or take effect until stdin EOF.
    • I'm used to typing Ctrl+C to get immediate interactivity on present input — but this works by killing the producer :-(.
    • It can do better on a file that's still being written! F tails
      seq 1 400000000 | grep --line-buffered 1234567 > TMP &
      sleep 1  # to avoid "Pattern not found"
      less +/77 +F
      
    • Ooh wow, a pipe with | less +F still shows input as it's coming, like regular less, but starts in F stdin-tailing mode! The difference is I can press Ctrl+X to get immediate interactivity (manpage says "on some systems", works for me on linux) — but without killing the producer; can later press F again to resume tailing stdin :tada:
    • Tailing with an active search also highlights matches as-they-are-coming :clap:.

I wasn't able to similate the magic F, Ctrl+X, search, F sequence using + flags. I'm curious whether | less --pattern=77 (or even regular | less) could always start in stdin-tailing mode?! Though I guess for now the less risky thing is introduce new command line option(s).

  • Hmm wait, / searching in stdin-tailing mode after Ctrl+X tends to get stuck until EOF? With or without --no-init. Even if there are matches to display in already-read portion, or on current screen. And it's the non-interruptible kind of stuck, Ctrl+X doesn't work. :neutral_face:

    What I'd want is combination of: (1) set "current pattern", but don't activate search yet (2) enter ESC-F mode, where matches get highlighted, yet stdin is being read as-it-comes, and interactivity is restored after a match (3) but without jumping to the end like F or ESC-F do. Move around same way as / and n and friends do.

    I guess improving interactive search to be interruptible can be done before thinking of cmdline flags.

Should I open a separate issue? [edit: #57 may be it? though that focuses on very-long-line case]

cben avatar Jan 23 '23 23:01 cben