Skip to content

Allow warning-count in status output to be styled#1675

Open
rolandwalker wants to merge 1 commit intomainfrom
RW/status-warning-count-style
Open

Allow warning-count in status output to be styled#1675
rolandwalker wants to merge 1 commit intomainfrom
RW/status-warning-count-style

Conversation

@rolandwalker
Copy link
Contributor

Description

Allow warning-count in status-footer output to have its own style, output.status.warning-count. Add a warnings.status.warning-count for completeness, though warnings styles remain undocumented for now.

output.status still has a style, and both styles are applied, so if output.status is italic, and output.status.warning-count is red, then the output of the warning count will be red+italic.

A status_plain property is added to SQLResult, since we are still often interested in the plain value, for string matches and calculating width.

Mycli.output now takes a SQLResult rather than a textual status.

A number of tests have to be updated to account for the above two structural changes, and one test assertion on the new style is added.

Example:

last image

This is definitely how I will prefer to consume warning notifications (vs \W), and it is again tempting to change the default.

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 .

@rolandwalker rolandwalker self-assigned this Mar 4, 2026
@github-actions
Copy link

github-actions bot commented Mar 4, 2026

  1. warnings.status.warning-count is currently not applied in warnings output path.
    In mycli/sqlexecute.py:405, warning count is always tagged as class:output.status.warning-count. But warnings rows are printed via MyCli.output(..., is_warnings_style=True), which applies class:warnings.status as the outer style. Result: users cannot target warnings.status.warning-count despite it being added in mycli/clistyle.py:49.
    Action: choose warning-count class based on warning context (or remap in output() when is_warnings_style=True) and add a test covering warnings-mode styling.

  2. SQLResult.status is typed as AnyFormattedText, but output() preserves formatting only for FormattedText.
    SQLResult.status now allows any prompt-toolkit formatted text, but main.py:1486-1489 falls back to plain text unless isinstance(status, FormattedText). That strips styles for other valid AnyFormattedText forms (e.g. tuple-lists/HTML), creating a subtle regression risk.
    Action: convert via to_formatted_text(result.status) unconditionally (for non-None) and test at least one non-FormattedText variant.

No direct security issues found in this diff.
I couldn’t run tests locally because pytest/uv are not available in this environment.

Allow warning-count in status-footer output to have its own style,
"output.status.warning-count".  Add a "warnings.status.warning-count"
for completeness, though warnings styles remain undocumented for now.

"output.status" still has a style, and both styles are applied, so if
"output.status" is italic, and "output.status.warning-count" is red,
then the output of the warning count will be red+italic.

A "status_plain" property is added to SQLResult, since we are still
often interested in the plain value, for string matches and calculating
width.

Mycli.output now takes a SQLResult rather than a textual "status".

A number of tests have to be updated to account for the above two
structural changes, and one test assertion on the new style is added.
@rolandwalker rolandwalker force-pushed the RW/status-warning-count-style branch from 6ddd69a to 9bbaf85 Compare March 4, 2026 12:24
Copy link
Contributor

@scottnemes scottnemes left a comment

Choose a reason for hiding this comment

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

Verified new formatting for warning count works as expected.

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