gwsw / less

Less - text pager
http://greenwoodsoftware.com/less
Other
561 stars 88 forks source link

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

Open ScoobyDone opened 5 years ago

ScoobyDone commented 5 years ago

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
$
gwsw commented 5 years ago

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.

cben commented 3 years ago

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

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 commented 1 year ago

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 commented 1 year ago

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:


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

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