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

lf can't preview long line strings #1446

Closed
gaesa opened this issue Sep 27, 2023 · 5 comments · Fixed by #1447
Closed

lf can't preview long line strings #1446

gaesa opened this issue Sep 27, 2023 · 5 comments · Fixed by #1447

Comments

@gaesa
Copy link

gaesa commented Sep 27, 2023

Hello,

I’ve encountered an issue where lf fails to display a line generated by a preview program when the line length reaches or exceeds 65536 characters. This issue is particularly noticeable when previewing compact JSON files.

To clarify, this seems to be an issue with lf, not the bat command used in the preview program. I’ve tested the bat --color=always -pp -- longline.txt command directly in the terminal, and it works fine. The long line is printed without any issues.

Version: 31

Operating System: Arch Linux

Installation Method: I installed lf using the command sudo pacman -S lf.

Steps to Reproduce:

  1. Set your preview program to preview a text file using the command: bat --color=always -pp -- your_file.
  2. Open lf and attempt to preview the file longline.txt.

Expected Behavior: lf should be able to display long line strings generated by a preview program, regardless of their length.

Actual Behavior: lf fails to display any line that contains 65536 characters or more.

@joelim-work
Copy link
Collaborator

This actually doesn't have anything to do with bat, I can reproduce the same issue using the default preview method.

The reason this happens is because lf uses bufio.Scanner to read the file line by line, which is limited by the size of its internal buffer MaxScanTokenSize. If a line exceeds the size of the buffer then the scanning terminates.

lf/nav.go

Lines 892 to 906 in dddf437

buf := bufio.NewScanner(reader)
for i := 0; i < win.h && buf.Scan(); i++ {
for _, r := range buf.Text() {
if r == 0 {
reg.lines = []string{"\033[7mbinary\033[0m"}
return
}
}
reg.lines = append(reg.lines, buf.Text())
}
if buf.Err() != nil {
log.Printf("loading file: %s", buf.Err())
}

@gaesa
Copy link
Author

gaesa commented Sep 27, 2023

I was wondering if it might be possible to consider an alternative approach. Instead of terminating the preview when a line exceeds the buffer size, perhaps lf could truncate the line and continue displaying it partially. This could potentially be accompanied by a subtle note or symbol indicating that the line has been truncated due to its length.

@joelim-work
Copy link
Collaborator

I agree that lines should be displayed even if they exceed the buffer length. Actually in this case there's no point in displaying any message since the preview window can only fit about 100 characters anyway, and you won't notice the difference.

Would you be able to try out the branch in https://github.com/joelim-work/lf/tree/preview-long-lines and see if it works for you? If it does then I might submit it as a PR.

@gaesa
Copy link
Author

gaesa commented Sep 27, 2023

I’ve tested the branch at https://github.com/joelim-work/lf/tree/preview-long-lines and I am content with the behaviour now. It works well and I agree there’s no need for a truncation message. Thank you for your efforts in addressing this issue.

@joelim-work
Copy link
Collaborator

Thanks, I have submitted PR #1447

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants