Thread (9 messages) flat view 9 messages, 4 authors, 2018-08-20

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help