git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Junio C Hamano <gitster@pobox.com>,
	Git Mailing List <git@vger.kernel.org>,
	bug-less@gnu.org
Subject: Re: "less -F" is broken
Date: Wed, 15 Aug 2018 23:29:28 +0200	[thread overview]
Message-ID: <87k1orqpxj.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CA+55aFzsVt9CJOBPGABcvg464W1THvwYpNhO+9DWUNw4X36Ndg@mail.gmail.com>


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

  parent reply	other threads:[~2018-08-15 21:29 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-15 20:35 "less -F" is broken Linus Torvalds
2018-08-15 21:23 ` Stefan Beller
2018-08-15 21:29 ` Ævar Arnfjörð Bjarmason [this message]
2018-08-15 21:43   ` Linus Torvalds
2018-08-15 21:57   ` Linus Torvalds
2018-08-16  8:22     ` Ævar Arnfjörð Bjarmason
2018-08-16 16:50     ` Mark Nudelman
2018-08-16 17:10       ` Linus Torvalds
2018-08-20 15:54         ` Mark Nudelman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k1orqpxj.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=bug-less@gnu.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).