Re: "less -F" is broken
From: Ævar Arnfjörð Bjarmason <hidden>
Date: 2018-08-15 21:29:34
On Wed, Aug 15 2018, Linus Torvalds wrote:
Sadly, as of less-530, the behavior of "less -F" is broken enough that I think git needs to potentially think about changing the defaults for the pager, or people should at least be aware of it.
Downloading & trying versions of it locally reveals that it's as of
version 520, not 530. The last version before 520 is 487. Presumably
it's covered by this item in the changelog:
Don't output terminal init sequence if using -F and file fits on one
screen[1]
Older versions of less (at least up to less-487 - March 2017) do not have this bug. There were apparently 520, 527 and 529 releases in 2017 too, but I couldn't find their sources to verify if they were already broken - but 530 (February 2018) has the problem.
FWIW they're not linked from http://www.greenwoodsoftware.com/less/download.html but you can just URL hack and see releases http://www.greenwoodsoftware.com/less/ and change links like http://www.greenwoodsoftware.com/less/less-530.tar.gz to http://www.greenwoodsoftware.com/less/less-520.tar.gz
The breakage is easy to see without git:
(echo "hello"; sleep 5; echo "bye bye") | less -F
which will result in no output at all for five seconds, and then you
get both lines at once as "less" exits.
The relevant change in less is this, cutting out the non-relevant parts:
diff --git a/less-487/forwback.c b/less-520/forwback.c
index 83ae78e..680fa25 100644
--- a/less-487/forwback.c
+++ b/less-520/forwback.c
[...]
@@ -444,3 +444,21 @@ get_back_scroll()
return (sc_height - 2);
return (10000); /* infinity */
}
+
+/*
+ * Return number of displayable lines in the file.
+ * Stop counting at screen height + 1.
+ */
+ public int
+get_line_count()
+{
+ int nlines;
+ POSITION pos = ch_zero();
+
+ for (nlines = 0; nlines <= sc_height; nlines++)
+ {
+ pos = forw_line(pos);
+ if (pos == NULL_POSITION) break;
+ }
+ return nlines;
+}
[...]
diff --git a/less-487/main.c b/less-520/main.c
index 960d120..6d54851 100644
--- a/less-487/main.c
+++ b/less-520/main.c
[...]
@@ -273,10 +275,19 @@ main(argc, argv)
{
if (edit_stdin()) /* Edit standard input */
quit(QUIT_ERROR);
+ if (quit_if_one_screen)
+ line_count = get_line_count();
} else
{
if (edit_first()) /* Edit first valid file in cmd line */
quit(QUIT_ERROR);
+ if (quit_if_one_screen)
+ {
+ if (nifile() == 1)
+ line_count = get_line_count();
+ else /* If more than one file, -F can not be used */
+ quit_if_one_screen = FALSE;
+ }
}
init();
diff --git a/less-487/screen.c b/less-520/screen.c
index ad3fca1..2d51bbc 100644
--- a/less-487/screen.c
+++ b/less-520/screen.c
[...]
@@ -1538,7 +1555,9 @@ win32_deinit_term()
init()
{
#if !MSDOS_COMPILER
- if (!no_init)
+ if (quit_if_one_screen && line_count >= sc_height)
+ quit_if_one_screen = FALSE;
+ if (!no_init && !quit_if_one_screen)
tputs(sc_init, sc_height, putchr);
if (!no_keypad)
tputs(sc_s_keypad, sc_height, putchr);
If you undo that first changed part in main.c your test case prints
"hello" to the terminal immediately.
It's not always obvious when using git, because when the terminal fills up, less also starts outputting, but the default options with -F are really horrible if you are looking for something uncommon, and "git log" doesn't respond at all. On the kernel tree, this is easy to see with something like git log --oneline --grep="The most important one is the mpt3sas fix" which takes a bit over 7 seconds before it shows the commit I was looking for. In contrast, if you do LESS=-RX git log --oneline --grep="The most important one is the mpt3sas fix" that (recent) commit is found and shown immediately. It still takes 7s for git to go through all history and decide "that was it", but at least you don't need to wait for the intermediate results. I've reported it as a bug in less, but I'm not sure what the reaction will be, the less releases seem to be very random.
Via bug-less@gnu.org? Is this report available online somewhere? Anyway, CC-ing that address since my digging into this will be useful to them. 1. http://www.greenwoodsoftware.com/less/news.520.html