Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement streaming preview window #2215

Merged
merged 15 commits into from
Oct 18, 2020
Merged

Implement streaming preview window #2215

merged 15 commits into from
Oct 18, 2020

Conversation

junegunn
Copy link
Owner

@junegunn junegunn commented Oct 16, 2020

Preliminary implementation of the streaming preview window. Please test and report any bugs. Fix #2212.

# Will start rendering after 200ms, update every 100ms
fzf --preview 'for i in $(seq 100); do echo $i; sleep 0.01; done'

# Should print "Loading preview" message after 500ms
fzf --preview 'sleep 1; for i in $(seq 100); do echo $i; sleep 0.01; done'

# The first line should appear after 200ms
fzf --preview 'date; sleep 2; date'

# Should not render before enough lines for the scroll offset are ready
rg --line-number --no-heading --color=always ^ |
  fzf --delimiter : --ansi --preview-window '+{2}-/2' \
      --preview 'sleep 1; bat --style=numbers --color=always --pager=never --highlight-line={2} {1}'

/ping @p-kolacz @edi9999

@junegunn junegunn self-assigned this Oct 16, 2020
@edi9999
Copy link
Contributor

edi9999 commented Oct 16, 2020

I see some issues on some of my custom scripts.

Only the first line is shown even after the preview has fully loaded.

When I start to scroll to the bottom, then I see the second line and the ones after that.

I'm trying to create a simple example.

@edi9999
Copy link
Contributor

edi9999 commented Oct 16, 2020

Actually it can be reproduced simply with the following :

FZF_DEFAULT_OPTS="" ./bin/fzf --preview 'echo "1"; sleep 1; echo 2'

@edi9999
Copy link
Contributor

edi9999 commented Oct 16, 2020

See recording :

record_2020-10-16_213457

@edi9999
Copy link
Contributor

edi9999 commented Oct 16, 2020

You can also reproduce the same issue (probably), with the following :

FZF_DEFAULT_OPTS="" ./bin/fzf --preview 'for i in $(seq 10); do echo $i; sleep 0.13; done' 

Here, on my computer, only the first 3 numbers are shown (numbers 1 to 3).

with :

FZF_DEFAULT_OPTS="" ./bin/fzf --preview 'for i in $(seq 10); do echo $i; sleep 0.11; done'

it writes number from 1 to 8.

@edi9999
Copy link
Contributor

edi9999 commented Oct 16, 2020

with FZF_DEFAULT_OPTS="" ./bin/fzf --preview 'for i in $(seq 10); do echo $i; sleep 0.10; done',

everything works fine, so there is something special happening near 1 tenth of a second.

@junegunn
Copy link
Owner Author

Nice find, 63444b4 should fix that.

@junegunn
Copy link
Owner Author

I'm also seeing an issue where fzf not terminating the running command when the cursor is moved. Need further investigation.

…ll offset

  rg --line-number --no-heading --color=always ^ |
    fzf --delimiter : --ansi --preview-window '+{2}-/2' \
        --preview 'sleep 1; bat --style=numbers --color=always --pager=never --highlight-line={2} {1}'
@edi9999
Copy link
Contributor

edi9999 commented Oct 17, 2020

There's one behavior I'm not sure is intended :

When I run the following :

fzf ./bin/fzf --preview ' yes | head -n 20; sleep 5; echo foo ;  ' --preview-window 'up:nohidden'

It shows the 1/20 while loading, even though the preview does not have a scroll.

After the sleep and the echo foo however, the 1/20 disappears.

I suggest to put only the loading screen in this case, not the 1/20 part.

Maybe to avoid some "jittering", it would make more sense to put the loading on the right of the pagination, or maybe one line below that.

@junegunn
Copy link
Owner Author

This is the intended behavior as of now. You can see there's a braille character indicating that the stream is open (i.e. ⠋ 1/20). We don't know if the final number of lines will exceed the height of the window or not at this point.

# `⠋ 1/20` disappears when the stream is closed
fzf --preview ' yes | head -n 20; sleep 5; echo foo ;  ' --preview-window 'up:nohidden'

# `1/1020` remains (spinner on the left gone)
fzf --preview ' yes | head -n 20; sleep 5; seq 1000; sleep 1' --preview-window 'up:nohidden'

One idea is to only print without numbers before the content overflows.

@edi9999
Copy link
Contributor

edi9999 commented Oct 17, 2020

One idea is to only print ⠋ without numbers before the content overflows.

Yes, exactly, that is what I thought would make more sense, but I also would understand that you'd prefer to keep current behavior.

@edi9999
Copy link
Contributor

edi9999 commented Oct 17, 2020

I have just tested again and it seems to work well for me.

@junegunn
Copy link
Owner Author

I'm going to merge this as it all seems to be working fine and test it over the next few days. Please let me know if you find any issues.

@junegunn junegunn merged commit faf68db into master Oct 18, 2020
@junegunn junegunn deleted the streaming-preview branch October 18, 2020 08:03
@edi9999
Copy link
Contributor

edi9999 commented Oct 18, 2020

Yes, sure I will !

kralicky pushed a commit to kralicky/fzf that referenced this pull request Jun 23, 2021
Fix junegunn#2212

    # Will start rendering after 200ms, update every 100ms
    fzf --preview 'for i in $(seq 100); do echo $i; sleep 0.01; done'

    # Should print "Loading .." message after 500ms
    fzf --preview 'sleep 1; for i in $(seq 100); do echo $i; sleep 0.01; done'

    # The first line should appear after 200ms
    fzf --preview 'date; sleep 2; date'

    # Should not render before enough lines for the scroll offset are ready
    rg --line-number --no-heading --color=always ^ |
      fzf --delimiter : --ansi --preview-window '+{2}-/2' \
          --preview 'sleep 1; bat --style=numbers --color=always --pager=never --highlight-line={2} {1}'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preview loading
2 participants