Skip to content

handle logic to auto close and delete brackets #501

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

Closed
lunargon wants to merge 4 commits into from
Closed

handle logic to auto close and delete brackets #501

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c8ac92e
handle logic to auto open and delete brackets
lunargon Jun 20, 2025
2e95f2e
remove check not useful
lunargon Jun 20, 2025
23518f5
improve auto delete both open and close
lunargon Jun 20, 2025
41b9408
clear part not use
lunargon Jun 25, 2025
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
80 changes: 77 additions & 3 deletions src/tui.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2308,9 +2308,27 @@ impl<'a> Context<'a, '_> {
}

let mut write: &[u8] = &[];

if let Some(input) = &self.input_text {
write = input.as_bytes();
// Handle auto-closing brackets
match *input {
"{" => {
self.handle_auto_closing_brackets(tb, "{", "}");
tc.preferred_column = tb.cursor_visual_pos().x;
}
"[" => {
self.handle_auto_closing_brackets(tb, "[", "]");
tc.preferred_column = tb.cursor_visual_pos().x;
}
"(" => {
self.handle_auto_closing_brackets(tb, "(", ")");
tc.preferred_column = tb.cursor_visual_pos().x;
}
_ => {
write = input.as_bytes();
tc.preferred_column = tb.cursor_visual_pos().x;
}
}
} else if let Some(input) = &self.input_keyboard {
let key = input.key();
let modifiers = input.modifiers();
Expand All @@ -2324,7 +2342,13 @@ impl<'a> Context<'a, '_> {
} else {
CursorMovement::Grapheme
};
tb.delete(granularity, -1);

// Check if we should delete both brackets when cursor is between them
if matches!(granularity, CursorMovement::Grapheme) && self.should_delete_both_brackets(tb) {
self.delete_both_brackets(tb);
} else {
tb.delete(granularity, -1);
}
}
vk::TAB => {
if single_line {
Expand Down Expand Up @@ -3320,8 +3344,58 @@ impl<'a> Context<'a, '_> {
self.block_begin("shortcut");
self.block_end();
}

self.attr_padding(Rect { left: 2, top: 0, right: 2, bottom: 0 });
}

/// Handles auto-closing brackets by inserting both opening and closing brackets
/// and positioning the cursor between them.
fn handle_auto_closing_brackets(&mut self, tb: &mut TextBuffer, opening: &str, closing: &str) {
Copy link

@karan68 karan68 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ impl::handle_auto_closing_brackets

Critical correctness issues: byte-length used for character-position cursor adjustment breaks non-ASCII brackets, and missing bounds check risks underflow panic. Also ignores I/O errors and lacks atomicity.

  • high: Byte/character mismatch: closing.len() returns byte length, but cursor logical positions (CoordType) typically measure Unicode scalar values or display columns. For multi-byte UTF-8 closing brackets (e.g., Unicode '」' is 3 bytes, 1 char), subtracting byte length moves cursor too far left, creating invalid or wrong positions. Use closing.chars().count() instead of closing.len().
  • high: Integer underflow risk: If the cursor x-position is smaller than closing.len() (e.g., at start of line), new_offset.x - closing.len() as CoordType panics in debug mode or wraps in release mode for unsigned CoordType. Must verify new_offset.x >= char_count before subtraction.
  • medium: Errors silently ignored: tb.write_raw() likely returns a Result indicating buffer full or I/O failures, but the code discards it with ;. On write failure, the cursor still moves to an incorrect calculated position, leaving the buffer in an inconsistent state.
  • low: Non-atomic operation: If the opening bracket write succeeds but the closing bracket write fails (or cursor move fails), the buffer ends up with an unmatched opening bracket without the corresponding closing bracket. Consider grouping changes in a transaction or undo block.

// Insert the opening bracket
tb.write_raw(opening.as_bytes());

// Insert the closing bracket
tb.write_raw(closing.as_bytes());

// Move cursor back to be between the brackets
// We need to move back by the length of the closing bracket
let new_offset = tb.cursor_logical_pos();
tb.cursor_move_to_logical(Point { x: new_offset.x - closing.len() as CoordType, y: new_offset.y });
}

/// Checks if we should delete both opening and closing brackets
///
/// Returns true if we should delete both brackets, false for normal deletion
fn should_delete_both_brackets(&mut self, tb: &mut TextBuffer) -> bool {
Copy link

@karan68 karan68 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 impl::should_delete_both_brackets

Implements bracket pair detection logic correctly for auto-delete functionality, but requires unnecessary mutable references that constrain the API. Also needs protection against negative cursor coordinates when casting to usize for the buffer read operations.

  • medium: Function signature uses &mut self and &mut TextBuffer but performs no mutation; should use &self and &TextBuffer instead. Unnecessary mutable references restrict API usage and prevent calling this predicate function when only immutable access is available
  • medium: Casting cursor_pos.x to usize without bounds checking risks integer overflow if cursor_pos.x is a signed type (e.g., i32) and negative. This could cause read_backward/read_forward to panic or behave incorrectly with an extremely large offset value

// Get the current cursor offset
let cursor_pos = tb.cursor_logical_pos();

// Read text before cursor (try to get at least 1 character)
let text_before = tb.read_backward(cursor_pos.x as usize);
let text_after = tb.read_forward(cursor_pos.x as usize);

// Check for common bracket pairs
if text_before.ends_with(b"(") && text_after.starts_with(b")") {
return true;
}
if text_before.ends_with(b"[") && text_after.starts_with(b"]") {
return true;
}
if text_before.ends_with(b"{") && text_after.starts_with(b"}") {
return true;
}

false
}

/// Deletes both opening and closing brackets when cursor is between them
fn delete_both_brackets(&mut self, tb: &mut TextBuffer) {
Copy link

@karan68 karan68 Mar 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💬 impl::delete_both_brackets

Helper function assumes cursor is positioned between brackets without validation. Consider adding checks to verify the cursor is bracketed by matching delimiters before deletion, or document preconditions clearly. Also verify TextBuffer::delete handles boundary conditions gracefully to avoid partial deletions.

  • medium: No validation that characters being deleted are actually brackets. If the cursor is not positioned between matching brackets (e.g., 'ab|cd'), this will delete arbitrary characters 'b' and 'c' instead of failing safely.
  • medium: Potential boundary issues: if the cursor is at the end of the buffer, the forward delete may be a no-op (depending on TextBuffer implementation), causing only the backward deletion to execute, or vice versa if at the start. This leaves the text buffer in an inconsistent state with only one character deleted.
  • low: Operation is not atomic. If the first delete succeeds but the second panics or fails, the buffer remains with only one bracket deleted, requiring manual cleanup.

// Delete the closing bracket first (forward)
tb.delete(CursorMovement::Grapheme, 1);
// Then delete the opening bracket (backward)
tb.delete(CursorMovement::Grapheme, -1);
}

}

/// See [`Tree::visit_all`].
Expand Down