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

Removing a bracket removes more than one bracket #4544

Closed
Jomy10 opened this issue Oct 31, 2022 · 2 comments · Fixed by #4558
Closed

Removing a bracket removes more than one bracket #4544

Jomy10 opened this issue Oct 31, 2022 · 2 comments · Fixed by #4558
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

Comments

@Jomy10
Copy link
Contributor

Jomy10 commented Oct 31, 2022

Summary

In this short video, I delete a closing bracket, but it also removes the closing bracket to the right of it.

Schermopname.2022-10-31.om.20.14.09.mov

Reproduction Steps

type something like (()) in helix. Remove the third bracket with backspace, the last one will disappear as well.

Helix log

No response

Platform

macOS

Terminal Emulator

iTerm2

Helix Version

helix 22.08.1 (f41f28b)
@Jomy10 Jomy10 added the C-bug Category: This is a bug label Oct 31, 2022
@David-Else
Copy link
Contributor

This problem trips me up often too.

@dead10ck dead10ck added E-easy Call for participation: Experience needed to fix: Easy / not much A-helix-term Area: Helix term improvements E-good-first-issue Call for participation: Issues suitable for new contributors labels Nov 1, 2022
ygabuev added a commit to ygabuev/helix that referenced this issue Nov 1, 2022
@ygabuev
Fix delete_char_backward for paired characters
23a2288 
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.

This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.

Closes helix-editor#4544.
@ygabuev ygabuev mentioned this issue Nov 1, 2022
Merged
@ygabuev
Copy link
Contributor

ygabuev commented Nov 1, 2022

#4558 is the change that fixes the issue.

However, there is still a problem with AutoPairs: they are annotated to be keyed by the opener:

helix/helix-core/src/auto_pairs.rs

Lines 19 to 22 in 8ff92c7

/// The type that represents the collection of auto pairs,
/// keyed by the opener.
#[derive(Debug, Clone)]
pub struct AutoPairs(HashMap<char, Pair>);

but the new() method also adds closers as keys:

helix/helix-core/src/auto_pairs.rs

Lines 90 to 91 in 8ff92c7

if auto_pair.open != auto_pair.close {
auto_pairs.insert(auto_pair.close, auto_pair);

This might be the issue why the bug was introduced in the first place - the contributor assumed that there are no closers in the list of keys of an AutoPairs instance.

archseer pushed a commit that referenced this issue Nov 1, 2022
@ygabuev
Fix delete_char_backward for paired characters (#4558)
c803ef8 
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.

This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.

Closes #4544.
pathwave pushed a commit to pathwave/helix that referenced this issue Nov 6, 2022
@ygabuev @pathwave
Fix delete_char_backward for paired characters (helix-editor#4558)
623fe24 
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.

This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.

Closes helix-editor#4544.
herkhinah pushed a commit to herkhinah/helix that referenced this issue Dec 11, 2022
@ygabuev
Fix delete_char_backward for paired characters (helix-editor#4558)
45a7a4a 
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.

This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.

Closes helix-editor#4544.
freqmod pushed a commit to freqmod/helix that referenced this issue Feb 8, 2023
@ygabuev
Fix delete_char_backward for paired characters (helix-editor#4558)
67d0279 
When backward-deleting a character, if this character and the following
character form a Pair, we want to delete both. However, there is a bug
that deletes both characters also if both characters are closers of some
Pair.

This commit fixes that by adding an additional check that the deleted
character should be an opener in a Pair.

Closes helix-editor#4544.
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
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix delete_char_backward for paired characters ygabuev/helix
4 participants
@dead10ck @David-Else @ygabuev @Jomy10