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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.