Skip to content

Trailing newlines not getting stripped from items #4282

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

Closed
5 of 10 tasks
alex-huff opened this issue Feb 24, 2025 · 11 comments
Closed
5 of 10 tasks

Trailing newlines not getting stripped from items #4282

alex-huff opened this issue Feb 24, 2025 · 11 comments

Comments

@alex-huff
Copy link
Contributor

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.60 (devel)

OS

  • Linux
  • macOS
  • Windows
  • Etc.

Shell

  • bash
  • zsh
  • fish

Problem / Steps to reproduce

Since commit 84e2262#diff-37113d7d967fa0fd21c0250890d9cda61c7d2591eeedd43017e9e7bc471b432aL138, newlines are no longer stripped from the end of each item when there are extra bytes (such as ANSI escape sequences) after the newline.

Before 84e2262:

fzfbefore-an.mp4

After:

fzfafter-an.mp4

Also, if you look at the after video I am moving back and forth between the first and second item and when I am focused on the second item the first item is no longer shown.

Here is a simple reproducing command:

printf "foo\n\e[0m\0bar\n\e[0m\0baz\n\e[0m\0" | ./fzf --read0 --ansi --with-nth ..

Before 84e2262:

Image

After:

Image

I will also try and find a reproducing command for the first item not showing.

@junegunn
Copy link
Owner

There was a regression. Does e4489dc fix the problem?

@alex-huff
Copy link
Contributor Author

There was a regression. Does e4489dc fix the problem?

No, I am currently on e4489dc

@alex-huff
Copy link
Contributor Author

fzfghost-an.mp4

Here is a reproducing command for the first item being truncated:

for _ in $(seq 19); do printf "foo\nbar\0"; done | ./fzf --read0

Change 19 so that the items no longer fit on screen.

@junegunn
Copy link
Owner

junegunn commented Feb 25, 2025

Since commit 84e2262#diff-37113d7d967fa0fd21c0250890d9cda61c7d2591eeedd43017e9e7bc471b432aL138, newlines are no longer stripped from the end of each item when there are extra bytes (such as ANSI escape sequences) after the newline.

I think the new behavior is correct here. fzf shouldn't trim the trailing new line without user's consent.

The second issue needs to be fixed, I'll look into it , but I don't think your patch is the correct way to approach the problem.

for _ in $(seq 5); do printf "foo\nbar\0"; done | fzf --read0 --height 11 --bind load:last

EDIT: Let me correct myself. I'll review your patch.

@junegunn
Copy link
Owner

Since commit 84e2262#diff-37113d7d967fa0fd21c0250890d9cda61c7d2591eeedd43017e9e7bc471b432aL138, newlines are no longer stripped from the end of each item when there are extra bytes (such as ANSI escape sequences) after the newline.

Can you show me the command you used here?

@alex-huff
Copy link
Contributor Author

alex-huff commented Feb 25, 2025

Can you show me the command you used here?

The command is massive because I wanted to support a bunch of weird edge-cases.
The full command that was used in those videos can be found here:
https://github.com/alex-huff/dotfiles/blob/main/dotfiles/.local/bin/rg-fzf

It relies on:
https://github.com/alex-huff/dotfiles/blob/main/dotfiles/.local/bin/rg-fzf-rg-wrapper.py

@alex-huff
Copy link
Contributor Author

alex-huff commented Feb 25, 2025

I think the new behavior is correct here. fzf shouldn't trim the trailing new line without user's consent.

I don't disagree with this and I can easily just update my script to omit trailing newlines. However, I think the following commands should produce visibly similar output.
Edit: the script seems to work fine when just removing \e[0m entirely. (similar to the second command below)

printf "foo\n\e[0m\0bar\n\e[0m\0baz\n\e[0m\0" | ./fzf --read0 --ansi --with-nth ..
printf "foo\n\0bar\n\0baz\n\0" | ./fzf --read0 --with-nth ..
printf "foo\n\0bar\n\0baz\n\0" | ./fzf --read0

It seems weird to me that a trailing newline will be stripped as long as ANSI escape sequences like \e[0m don't exist after it. It also seems weird that --with-nth .. causes newlines to be stripped but without it they are kept.

@junegunn
Copy link
Owner

--with-nth is an option that commands fzf to transform the input and it removes the trailing whitespaces after transforming it. It has been so for quite some time, and this is indeed a desirable behavior in many common scenarios. Multi-line support is a very recent addition to fzf, so this new-line trimming case was definitely not something we considered back then.

Two things.

  • Maybe --with-nth .. should be a no-op. OR, if the user knows that it removes the trailing whitespaces, they can take advantage of it to just remove trailing whitespaces.
  • The first case where --with-nth .. failed to remove \n is not an expected behavior. We should fix it, but it can be quite tricky to get it right. When tokenizing the fields for transforming the input, fzf doesn't trim ANSI escape sequences, because the we want to keep the colors that are stateful, and the "trimming" we mentioned above happens here, and because there is \e[0m, fzf cannot remove new line before it.

@alex-huff
Copy link
Contributor Author

alex-huff commented Feb 25, 2025

  • The first case where --with-nth .. failed to remove \n is not an expected behavior. We should fix it, but it can be quite tricky to get it right. When tokenizing the fields for transforming the input, fzf doesn't trim ANSI escape sequences, because the we want to keep the colors that are stateful, and the "trimming" we mentioned above happens here, and because there is \e[0m, fzf cannot remove new line before it.

It worked before by calling the now removed item.text.TrimTrailingWhitespaces() after processing ANSI stuff. Was this removed for a reason?

@junegunn
Copy link
Owner

Good point. It was removed while implementing --accept-nth and factoring out the transformation logic. I'll see what I can do.

@junegunn
Copy link
Owner

So TrimTrailingWhitespaces is back. Although the question of whether we should remove the trailing new lines or not in multi-line mode remains, we're back to the old behavior.

tmeijn pushed a commit to tmeijn/dotfiles that referenced this issue Mar 4, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [junegunn/fzf](https://github.com/junegunn/fzf) | patch | `v0.60.2` -> `v0.60.3` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>junegunn/fzf (junegunn/fzf)</summary>

### [`v0.60.3`](https://github.com/junegunn/fzf/releases/tag/v0.60.3): 0.60.3

[Compare Source](junegunn/fzf@v0.60.2...v0.60.3)

-   Bug fixes and improvements
    -   \[fish] Enable multiple history commands insertion ([#&#8203;4280](junegunn/fzf#4280)) ([@&#8203;bitraid](https://github.com/bitraid))
    -   \[walker] Append '/' to directory entries on MSYS2 ([#&#8203;4281](junegunn/fzf#4281))
    -   Trim trailing whitespaces after processing ANSI sequences ([#&#8203;4282](junegunn/fzf#4282))
    -   Remove temp files before `become` when using `--tmux` option ([#&#8203;4283](junegunn/fzf#4283))
    -   Fix condition for using item numlines cache ([#&#8203;4285](junegunn/fzf#4285)) ([@&#8203;alex-huff](https://github.com/alex-huff))
    -   Make `--accept-nth` compatible with `--select-1` ([#&#8203;4287](junegunn/fzf#4287))
    -   Increase the query length limit from 300 to 1000 ([#&#8203;4292](junegunn/fzf#4292))
    -   \[windows] Prevent fzf from consuming user input while paused ([#&#8203;4260](junegunn/fzf#4260))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xODUuMCIsInVwZGF0ZWRJblZlciI6IjM5LjE4NS4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6WyJSZW5vdmF0ZSBCb3QiXX0=-->
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

No branches or pull requests

2 participants