Skip to content

Comments

Strip trailing newlines with \e <filename>#1613

Merged
rolandwalker merged 1 commit intomainfrom
RW/uniformly-strip-edit-command-newlines
Feb 24, 2026
Merged

Strip trailing newlines with \e <filename>#1613
rolandwalker merged 1 commit intomainfrom
RW/uniformly-strip-edit-command-newlines

Conversation

@rolandwalker
Copy link
Contributor

Description

Trailing newlines were stripped in other variations of \e, but not when reading from a file. Stripping helps place the cursor in the expected place when returning to the prompt.

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 .

@github-actions
Copy link

Review summary
No correctness or security regressions found in the code change itself. One test gap.

Findings

  1. Missing test coverage for the \e <filename> branch that now strips trailing newlines. This is the only behavior change in the PR and isn’t exercised by existing tests. Add a test in test/test_special_iocommands.py that writes a temp file with a trailing newline (and ideally multiple) and asserts open_external_editor(filename=...) returns the content without trailing \n. This should be gated to non-Windows like the other editor test, or account for \r\n.
    File: mycli/packages/special/iocommands.py:188

If you want, I can draft the test.

@rolandwalker rolandwalker self-assigned this Feb 23, 2026
Trailing newlines were stripped in other variations of "\e", but not
when reading from a file.  Stripping helps place the cursor in the
expected place when returning to the prompt.
@rolandwalker rolandwalker force-pushed the RW/uniformly-strip-edit-command-newlines branch from 74700fb to f061131 Compare February 24, 2026 09:54
@rolandwalker rolandwalker merged commit 98ed156 into main Feb 24, 2026
13 of 14 checks passed
@rolandwalker rolandwalker deleted the RW/uniformly-strip-edit-command-newlines branch February 24, 2026 10:00
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