Skip to content

Fix issue stripping multi-character delimiters#1765

Merged
rolandwalker merged 1 commit intomainfrom
RW/fix-multichar-delimiter-issue
Apr 1, 2026
Merged

Fix issue stripping multi-character delimiters#1765
rolandwalker merged 1 commit intomainfrom
RW/fix-multichar-delimiter-issue

Conversation

@rolandwalker
Copy link
Copy Markdown
Contributor

Description

  • strip by the whole delimiter, not the character/s
  • strip only from the end
  • add tests for delimitercommand.py

Checklist

  • I added this contribution to the changelog.md file.
  • I added my name to the AUTHORS file (or it's already there).
  • To lint and format the code, I ran
    uv run ruff check && uv run ruff format && uv run mypy --install-types .

 * strip by the whole delimiter, not the character/s
 * strip only from the end
 * add tests for delimitercommand.py
@rolandwalker rolandwalker self-assigned this Mar 31, 2026
@github-actions
Copy link
Copy Markdown

Codex Review

No correctness or security issues found in the PR changeset (9432311).

The core fix in mycli/packages/special/delimitercommand.py is correct: replacing strip(delimiter) with suffix slicing prevents accidental removal of delimiter characters from the start of statements when delimiters are multi-character.

Test coverage is improved with targeted regression tests in test/pytests/test_delimitercommand.py, including the multi-character delimiter case and delimiter-switch flow.

Residual risk: I couldn’t execute tests in this environment because uv is unavailable (uv: command not found), so this is a static review only.

@rolandwalker rolandwalker requested a review from scottnemes March 31, 2026 22:39
Copy link
Copy Markdown
Contributor

@j-bennet j-bennet left a comment

Choose a reason for hiding this comment

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

Nice!

@rolandwalker rolandwalker merged commit a7f15e4 into main Apr 1, 2026
10 checks passed
@rolandwalker rolandwalker deleted the RW/fix-multichar-delimiter-issue branch April 3, 2026 10:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants