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.

Sign up for GitHub

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

Jump to bottom

Split Bug at Start of Line #3850

Open
n0s4 opened this issue Sep 15, 2022 · 2 comments · May be fixed by #7854
Open

Split Bug at Start of Line #3850

n0s4 opened this issue Sep 15, 2022 · 2 comments · May be fixed by #7854
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug 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

@n0s4
Copy link
Contributor

n0s4 commented Sep 15, 2022

Summary

When Split is used with a pattern that matches the start of a line, a selection is placed on the match even when there aren't any characters before it:

https://asciinema.org/a/NDCiZr7kfsMiUEuI88K37vybT

Reproduction Steps

I tried this:

  1. hx
  2. i| | | |(esc)S\|(ret)

I expected this to happen:
The spaces in between and the newline would be selected

Instead, this happened:
There was an additional selection at the start of the line.

Helix log

N/A

Platform

Linux

Terminal Emulator

Alacritty

Helix Version

22.08.1-61-gb191ecf8
@n0s4 n0s4 added the C-bug Category: This is a bug label Sep 15, 2022
@kirawi kirawi added the A-helix-term Area: Helix term improvements label Sep 15, 2022
@the-mikedavis
Copy link
Member

This looks like #3544. The same fix from https://github.com/helix-editor/helix/pull/3598/files can probably be applied to split_on_matches

@pascalkuthe pascalkuthe added 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 labels Jul 20, 2023
@the-mikedavis the-mikedavis linked a pull request Aug 7, 2023 that will close this issue
Open
@palango
Copy link
Contributor

palango commented Dec 13, 2023

I think there are some edge cases that need to be decided. For example, on the input |a|b|c|, and a split on | what would be the expected outcome: three or four selections?

For me it feels more natural to have three, but have the special case of having the first selection remaining if there is no match. I implemented this in palango@17d1cd3#diff-f218cebd639a867087e2a7abc84160d45888ad5bcff4ab10fcf7217cb16586e9R1303 , however the code isn't optimized yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug 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
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix unexpected first selection when splitting brendonh8/helix
5 participants
@palango @the-mikedavis @pascalkuthe @kirawi @n0s4