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

Cannot replace newline with r #10725

Open
knuesel opened this issue May 10, 2024 · 4 comments · May be fixed by #10786
Open

Cannot replace newline with r #10725

knuesel opened this issue May 10, 2024 · 4 comments · May be fixed by #10786
Labels
A-command Area: Commands C-bug Category: This is a bug C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR

Comments

@knuesel
Copy link

knuesel commented May 10, 2024

I love how newline are treated largely like normal characters. One thing that doesn't seem to work though is replacing them with the r key (tested using Helix 24.03):

When the cursor is on a newline, pressing r, doesn't have the expected effect of joining the lines with a comma separator.

With a larger selection, pressing s\n and Enter selects the newlines as expected, but r, doesn't replace them.

I could use Alt-J before r to first convert the newlines to space but it seems like the direct way should just work?

@the-mikedavis the-mikedavis added C-enhancement Category: Improvements A-command Area: Commands labels May 10, 2024
@dead10ck
Copy link
Member

Can confirm, it looks like r doesn't work at all when it's on the line feed

@dead10ck dead10ck added the C-bug Category: This is a bug label May 10, 2024
@the-mikedavis
Copy link
Member

Looks like this was introduced intentionally a while ago in #167 without a strong preference. I would prefer that we don't special case line endings. You can always shrink or split a selection before replacing if you don't want to replace the line endings

@kirawi kirawi added the E-easy Call for participation: Experience needed to fix: Easy / not much label May 13, 2024
@kirawi
Copy link
Member

kirawi commented May 13, 2024

I'm not sure if that code was changed later on, but I thing it could also lead to different behavior on other line endings since it's hardcoded to LF?

@kirawi kirawi added the E-good-first-issue Call for participation: Issues suitable for new contributors label May 13, 2024
@kirawi
Copy link
Member

kirawi commented May 14, 2024

I think the fix is to remove the check for a line ending here

if str_is_line_ending(&cow) {
cow
} else {
ch.into()
}

@kirawi kirawi added the E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR label May 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-command Area: Commands C-bug Category: This is a bug C-enhancement Category: Improvements E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors E-has-instructions Call for participation: Has instructions for fixing the issue and opening a PR
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants