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