page-up/down doesn’t take into account the number of lines of items in mult-line mode

[vejkse]

Checklist

• I have read through the manual page (man fzf)
• I have searched through the existing issues
• For bug reports, I have checked if the bug is reproducible in the latest version of fzf

Output of fzf --version

0.56 (devel)

OS

• Linux
• macOS
• Windows
• Etc.

Shell

• bash
• zsh
• fish

Problem / Steps to reproduce

With one-line items, when doing PgUp (bound by default to page-up) and PgDn (bound by default to page-down), if 21 lines (and so 21 items) are visible as in the following example:

for i in {01..99}; do echo -ne "item $i\0"; done | fzf --read0 --reverse --height=23

each movement one page up or down goes 20 lines (and so 20 items) up or down the list, as one would expect.

But in the following example with 2-line items, where 21 lines (and so 10.5 items) are visible:

for i in {01..99}; do echo -ne "item $i, line 1\nitem $i, line 2\0"; done | fzf --read0 --reverse --height=23

each movement one page up or down doesn’t go about 20 lines up or down the list, as one would expect, but 20 items (so 40 lines) up or down the list, so that each time we go up or down, we skip about 10 items.

[junegunn]

Thanks for the report. It should be fixed now on master.

[vejkse]

Thank you!

But I think it doesn’t work in all cases. I should have made an example with variable length, like this one where each item n > 1 has ⌈n/2⌉ + 1 lines:

for i in {1..99}; do printf "item $i, line %d\n" $(seq 1 $((i/2))); echo -ne "item $i, line $((i/2+1))\0"; done | fzf --read0 --reverse --height=23

Here is how it goes when repeating page-down:

1. Items 1-7 are fully visible and 1 line of item 8.
2. Items 5-9 are fully visible.
3. Items 11-13 are fully visible and 1 line of item 14 — so item 10 was jumped over.
4. Items 14-15 are fully visible and 5 lines of item 16.
5. Items 18-19 are fully visible and 1 line of item 20 — so item 17 was jumped over.

After that, we see all items but some only partially even though they could be fully visible if from that point on, page-down became equivalent to down.

Of course, once we get to items that have too many lines to be shown in that limited height, they will never be fully visible.

[vejkse]

Even a uniform 3-line case has a little problem, like this one:

for i in {01..99}; do echo -ne "item $i, line 1\nitem $i, line 2\nitem $i, line 3\0"; done | fzf --read0 --reverse --height=22

When repeating page-down, after the initial steps, only two lines of the last item on each page are visible, but the full item is not visible on the next page. One could argue that this is acceptable, I suppose.

[junegunn]

Thanks for the repro.

Turns out it's not quite trivial to get it right, especially when considering the different values of --scroll-off. Will revisit this later.

[junegunn]

See 4a85843.

The behavior of the actions could still be improved, but at least they don't skip items.

[vejkse]

Thank you! But I’m afraid there are still cases where items are skipped… This looks very tricky.

Here is one:

{ for i in {1..30}; do printf "item $i, line 1\0"; done; for i in {31..34}; do printf "item $i, line 1\n         line 2\n         line 3\0"; done; for i in {35..90}; do printf "item $i, line 1\0"; done; } | fzf --read0 --reverse --height=28

When iterating page-down, we get:

1. items 1-26;
2. items 4-29;
3. items 29-46 (the only few multi-line items appear here);
4. items 51-76 (so 47-50 were skipped).

I’m wondering if the problem doesn’t come from the fact that when moving down one item, the number of lines by which the ‘window’ is moved depends not on the number of lines of the current or next item, but on the number of lines of the first item in the ‘window’?

[junegunn]

Thanks for finding the edge cases and sharing the insight. I think I'm going to have to take a different approach to the problem. I'll let you know of the progress.

[junegunn]

I'd love to hear your feedback on 9c94f9c. Thanks.

[vejkse]

It often works perfectly, but it can still skip items. I noticed it happening when, after doing page-down I land on an abnormally long multiline item.

Here’s an example:

{ for i in {1..20}; do printf "item $i, line 1\0"; done; printf "item 21, line 1\n         line 2\n         line 3\n         line 4\0"; for i in {22..60}; do printf "item $i, line 1\0"; done; } | fzf --read0 --reverse --height=13

All items have one line, apart from item 21 that has 4 lines. When repeating page-down, we get:

1. items 1-11, item 1 selected;
2. items 4-14, item 11 selected;
3. items 17-24, the 4-line item 21 selected (items 15-16 were skipped).

In fact, when doing down repeatedly, we get (I skip the beginning):

1. items 11-21 (only 1 line), item 18 selected;
2. items 12-21 (only 2 lines), item 19 selected;
3. items 13-21 (only 3 lines), item 20 selected;
4. a jump: items 17-24, item 21 selected.

I suppose this jump leads to the skipped items when doing page-down.

This would then be a problem with down, since to keep 3 lines below, the 4th step just above should show items 14-21, item 21 selected (with all 4 lines visible, so there are 3 lines below). The jump would occur at the next step, then (hopefully without leading to skipped items in page-down).

[vejkse]

I realize now there is no problem with down, since what is kept is 3 lines below the full item rather than below the first line of the item.

Also, there was a coincidence in my example above that would not be typical: item 21 has 4 (= 1 + 3) lines and the (default) value of --scroll-off is 3.

With 6 lines for item 21:

{ for i in {1..20}; do printf "item $i, line 1\0"; done; printf "item 21, line 1\n         line 2\n         line 3\n         line 4\n         line 5\n         line 6\0"; for i in {22..60}; do printf "item $i, line 1\0"; done; } | FZF_DEFAULT_OPTS= fzf --read0 --reverse --height=13

the 4th step above would still need a jump to maintain (at least) 3 lines below, even if it was below the first line of item 21 rather than below its last line.

With page-down, in the third step, item 20 should be selected to maintain the 3 lines below without jumping over some items.

[junegunn]

Thank you very much again, you have great attention to detail.

So these page actions are internally translated to a series of downs and ups and there are cases where a single down or up action can skip multiple items breaking the minimum/maximum offset condition. So I think we can fix it by undoing the last action if the condition is broken by it. Let me see.

[junegunn]

Here's another take. Please take a look.

[vejkse]

It looks almost perfect! I did not manage to make it skip any item. I still found one imperfection, though, and I’m not entirely sure it could not be exploited to skip an item.

In the following example:

#!/bin/bash

lines=(
  [50]=23
  [51]=23
)

print_item() {
  num_of_item="$1"
  num_of_lines="$2"
  printf 'item %3d, line 1' "$num_of_item"
  for l in $(seq 2 "$num_of_lines"); do
    printf '\n          line %s' "$l"
  done
  printf '\0'
}

for i in {1..120}; do
  print_item "$i" "${lines[i]:-1}"
done | FZF_DEFAULT_OPTS='' fzf --read0 --reverse --scroll-off=6 --wrap --height=30

only items 50 and 51 have multiple lines (23 each). If we repeat page-down, we get:

1. Items 1-28 (item 1 selected).
2. Items 7-34 (item 28 selected).
3. Items 28-50 (item 50 selected, only the first 6 lines are visible).
4. Items 46-51 (item 51 selected, only the first line is visible).
5. Items 52-79 (item 52 selected).
6. Items 58-85 (item 79 selected).

So we only got to see 1 line out of 23 of item 51.

When repeating down from step 2 above, we see that everything goes as expected until:

1. Items 28-50 (item 49 selected, 6 lines of item 50 visible).
2. Items 28-50 (item 50 selected, still only 6 lines visible). I would have expected item 50 to become fully visible (without a margin of 6 lines above and below, since there is no space enough for that).

By playing a bit with the values, I think this can only happen when the difference between the total number of lines and the number of lines of the item in question is less than the --scroll-off.

[junegunn]

Thank you! For now, I'm going to wrap this issue up here as fzf is no longer skipping items which I think was a bigger problem. The issue you raised above needs to be addressed as well, but I believe it's affecting fewer people and it's not easy to reconcile the different options without making the code too complicated. Anyway, I'll let you know when I revisit this. Thank you again for your help.

[vejkse]

Thank you for your work! The last issue is probably more a problem with down than with page-down and it won’t happen very often.

[vejkse]

I’m sorry, but I have bad news: there is still a problem at the very end (the last page), where abnormal behavior can occur, but only when --wrap is on, even if there is no line to wrap.

In this example (with no line to wrap):

{ for i in {1..25}; do printf "item $i, line 1\0"; done; printf 'item 26, line 1\n         line 2\n         line 3\n         line 4\0'; for i in {27..58}; do printf "item $i, line 1\0"; done; } | fzf --read0 --reverse --height=35 --wrap

we get the following behaviour:

1. At the beginning we see items 1-30, item 1 selected.
2. After one page-down, we instantaneously see items 7-36, item 33 selected.
3. After a second page-down, after a sensible delay, we see items 26-55, with no visible item selected.
4. Further page-downs don’t have any effect. But doing down once leads to seeing items 29-58, with item 58 selected.
5. Doing down again leads back to the state after step 3. Doing up repeatedly leads to oscillations.

If we remove --wrap (or toggle it), everything is fine. If we add --scroll-off=12, it seems to me that the delay is more sensible.

[junegunn]

Nice find, thanks! It looks like a separate issue that I can reproduce with just down. I'll open a new issue referencing your comment.