Skip to content

Make prettify/unprettify logic more robust#1646

Merged
rolandwalker merged 1 commit intomainfrom
RW/more-robust-prettify-logic
Feb 28, 2026
Merged

Make prettify/unprettify logic more robust#1646
rolandwalker merged 1 commit intomainfrom
RW/more-robust-prettify-logic

Conversation

@rolandwalker
Copy link
Contributor

Description

  • explicitly handle more exceptional cases such as empty input
  • clarify in error message that only a single statement is expected
  • don't return an empty string on failure; instead return the original text

Works together with #1645.

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

  1. Potential regression: failure path mutates input instead of returning original text
    In both methods, parse failure now does text.rstrip(';') rather than returning the original text. This strips all trailing semicolons and can produce '' for inputs like ';'/';;;', which conflicts with the PR goal (“return the original text on failure”).

  2. State-coupling bug risk: semicolon append depends on global error state
    Successful parse appends ; only when not self.toolbar_error_message. If an earlier failure left this flag set, a later successful prettify/unprettify can skip semicolon append unexpectedly.

    • mycli/main.py:859
    • mycli/main.py:875
      Action: use a local parse_failed flag (or clear toolbar_error_message at method start / in success branch) so success formatting is independent of stale UI state.

Missing tests / edge cases

  • Add tests for failure-path exact preservation of input (';', ';;;', 'SELECT 1;;', unparsable SQL).
  • Add test for “failure then success” on same MyCli instance to ensure success still appends semicolon and error state is reset appropriately.

No direct security concerns found in this PR diff.
I did not run tests here because required runtime deps (e.g., sqlglot) are not available in this environment.

@rolandwalker rolandwalker force-pushed the RW/more-robust-prettify-logic branch from 2c8ef5c to fa2a939 Compare February 27, 2026 11:47
  * explicitly handle more exceptional cases such as empty input
  * clarify in error message that only a single statement is expected
  * don't return an empty string on failure; instead return the
    original text
@rolandwalker rolandwalker force-pushed the RW/more-robust-prettify-logic branch from fa2a939 to cbfb566 Compare February 28, 2026 11:33
@rolandwalker
Copy link
Contributor Author

Merging this without review, since it is pretty much required for #1645, which I merged without noticing the state of this one.

@rolandwalker rolandwalker merged commit e0d13a1 into main Feb 28, 2026
8 checks passed
@rolandwalker rolandwalker deleted the RW/more-robust-prettify-logic branch February 28, 2026 11:39
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.

1 participant