Skip to content

Fix auto deletion logic #929

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

Open
HalFrgrd wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix auto deletion logic #929

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions src/app/auto_close.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,16 @@ pub(crate) fn delete_auto_inserted_closing_if_present(
return;
}

// Fallback for parser edge cases (notably consecutive `(` that can be tokenized
// as arithmetic-command boundaries): if the cursor is directly between `(` and `)`,
// delete the right paren first so Backspace removes the innermost pair.
// Fallback for parser edge cases: if the cursor is directly between any opening
// character and its matching closing character, delete the right char first so
// Backspace removes the pair.
if cursor_pos > 0 {
let left_char = buffer.buffer()[..cursor_pos].chars().next_back();
let right_char = buffer.buffer()[cursor_pos..].chars().next();
if left_char == Some('(') && right_char == Some(')') {
if let Some(left) = left_char
&& let Some(expected_closing) = surround_closing_char(left)
&& right_char == Some(expected_closing)
{
buffer.delete_right();
}
}
Expand Down Expand Up @@ -441,4 +444,37 @@ mod tests {
assert_eq!(buffer.cursor_byte_pos(), 5);
let _ = tokens;
}

#[test]
fn brace_autoclose_in_path_argument_and_backspace() {
let mut buffer = TextBuffer::new("asdf ./docker/");
let mut tokens = parsed(buffer.buffer());

handle_char_insertion(&mut buffer, &mut tokens, '{');

assert_eq!(buffer.buffer(), "asdf ./docker/{}");
assert_eq!(buffer.cursor_byte_pos(), 15);

delete_auto_inserted_closing_if_present(&mut buffer, &tokens);
buffer.delete_left();
tokens = dparser::DParser::parse_and_transfer_auto_inserted_flags(buffer.buffer(), &tokens);

assert_eq!(buffer.buffer(), "asdf ./docker/");
assert_eq!(buffer.cursor_byte_pos(), 14);
let _ = tokens;
}

#[test]
fn brace_autoclose_in_path_argument_and_overwrite() {
let mut buffer = TextBuffer::new("asdf ./docker/");
let mut tokens = parsed(buffer.buffer());

handle_char_insertion(&mut buffer, &mut tokens, '{');
assert_eq!(buffer.buffer(), "asdf ./docker/{}");
assert_eq!(buffer.cursor_byte_pos(), 15);

handle_char_insertion(&mut buffer, &mut tokens, '}');
assert_eq!(buffer.buffer(), "asdf ./docker/{}");
assert_eq!(buffer.cursor_byte_pos(), 16);
}
}
95 changes: 56 additions & 39 deletions src/grammar/dparser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,10 +967,12 @@ impl DParser {
.find(|t| t.token.byte_range().contains(&cursor_pos))
&& let Some(closing) = dparser_token.annotations.closing.as_mut()
&& closing.is_auto_inserted
&& dparser_token.token.value.starts_with(c)
{
closing.is_auto_inserted = false;
return true;
let offset = cursor_pos - dparser_token.token.byte_range().start;
if dparser_token.token.value[offset..].starts_with(c) {
closing.is_auto_inserted = false;
return true;
}
}

false
Expand Down Expand Up @@ -1013,34 +1015,47 @@ impl DParser {
}

// Fallback for lexer/parser edge cases where no structural opening/closing annotation
// is available (e.g. consecutive `(` merged into ArithCommand tokens). If the cursor is
// exactly between an opening char and an auto-inserted matching closing token, delete it.
let Some(opening_char) = opening_token.token.value.chars().next_back() else {
// is available (e.g. consecutive `(` merged into ArithCommand tokens, or `{` / `[` inside words).
let offset = cursor_pos - 1 - opening_token.token.byte_range().start;
let Some(opening_char) = opening_token.token.value[offset..].chars().next() else {
return false;
};
let Some(expected_closing_char) = surround_closing_char(opening_char) else {
return false;
};

opening_token.token.byte_range().end == cursor_pos
&& tokens.iter().any(|closing_token| {
let is_empty_arith_command_pair = opening_token.token.kind
== TokenKind::ArithCommand
&& matches!(
closing_token.token.kind,
TokenKind::DoubleRParen | TokenKind::RParen
)
&& closing_token.token.value.starts_with(')');
// Case A: closing token is the same token (e.g. `./docker/{}` where both `{` and `}` are in the word token)
if opening_token.token.byte_range().contains(&cursor_pos) {
let close_offset = cursor_pos - opening_token.token.byte_range().start;
if opening_token.token.value[close_offset..].starts_with(expected_closing_char)
&& opening_token
.annotations
.closing
.as_ref()
.is_some_and(|closing| closing.is_auto_inserted)
{
return true;
}
}

closing_token.token.byte_range().start == cursor_pos
&& closing_token.token.value.starts_with(expected_closing_char)
&& (closing_token
.annotations
.closing
.as_ref()
.is_some_and(|closing| closing.is_auto_inserted)
|| is_empty_arith_command_pair)
})
// Case B: closing token is a separate token
tokens.iter().any(|closing_token| {
let is_empty_arith_command_pair = opening_token.token.kind == TokenKind::ArithCommand
&& matches!(
closing_token.token.kind,
TokenKind::DoubleRParen | TokenKind::RParen
)
&& closing_token.token.value.starts_with(')');

closing_token.token.byte_range().start == cursor_pos
&& closing_token.token.value.starts_with(expected_closing_char)
&& (closing_token
.annotations
.closing
.as_ref()
.is_some_and(|closing| closing.is_auto_inserted)
|| is_empty_arith_command_pair)
})
}

pub fn mark_auto_inserted_closing(
Expand All @@ -1049,22 +1064,24 @@ impl DParser {
byte_pos: usize,
) -> bool {
for token in tokens {
if token.token.byte_range().start == byte_pos && token.token.value.starts_with(c) {
if let Some(closing) = &mut token.annotations.closing {
closing.is_auto_inserted = true;
} else {
// The token kind is not paired by the dparser nesting machinery
// (e.g. `]`, which is intentionally not a nesting closer because
// `[` does not start a nesting). Synthesize a closing annotation
// purely so the auto-close machinery can recognise this token
// as auto-inserted on the next keystroke. `opening_idx` is unused
// for non-nested closers and is set to 0 as a placeholder.
token.annotations.closing = Some(ClosingAnnotation {
opening_idx: 0,
is_auto_inserted: true,
});
if token.token.byte_range().contains(&byte_pos) {
let offset = byte_pos - token.token.byte_range().start;
if token.token.value[offset..].starts_with(c) {
if let Some(closing) = &mut token.annotations.closing {
closing.is_auto_inserted = true;
} else {
// The token kind is not paired by the dparser nesting machinery
// (e.g. `]`, or `{` inside a word). Synthesize a closing annotation
// purely so the auto-close machinery can recognise this token
// as auto-inserted on the next keystroke. `opening_idx` is unused
// for non-nested closers and is set to 0 as a placeholder.
token.annotations.closing = Some(ClosingAnnotation {
opening_idx: 0,
is_auto_inserted: true,
});
}
return true;
}
return true;
}
}

Expand Down