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

--pattern overrides -F and tildes up the screen #28

Open
ScoobyDone opened this issue Oct 14, 2019 · 4 comments
Open

--pattern overrides -F and tildes up the screen #28

ScoobyDone opened this issue Oct 14, 2019 · 4 comments

Comments

@ScoobyDone
Copy link

ScoobyDone commented Oct 14, 2019

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 ScoobyDone changed the title --pattern overrides -F and tildes up the screen <deleted> sigh Oct 14, 2019
@ScoobyDone ScoobyDone changed the title <deleted> sigh --pattern overrides -F and tildes up the screen Oct 14, 2019
@gwsw
Copy link
Owner

gwsw commented Oct 20, 2019

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 gwsw added the question label Dec 25, 2020
@cben
Copy link

cben commented Feb 17, 2021

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.

@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?

@gwsw
Copy link
Owner

gwsw commented Jan 20, 2023

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).

@cben
Copy link

cben commented Jan 23, 2023

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. 👏

  • But searching e.g. | less --pattern=77 breaks that, it waits until EOF. ⏳
  • | 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) ❔
  • 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 🎉
    • Tailing with an active search also highlights matches as-they-are-coming 👏.

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. 😐

    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]

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

3 participants