Skip to content

Clean up error output: color reset sequence and stat failure message - #482

Open
ammachado wants to merge 1 commit into
psss:mainfrom
ammachado:did-error-output
Open

Clean up error output: color reset sequence and stat failure message#482
ammachado wants to merge 1 commit into
psss:mainfrom
ammachado:did-error-output

Conversation

@ammachado

Copy link
Copy Markdown
Contributor

Two fixes

did/utils.py

The reset sequence emitted by color() was \033[1;m, where it should be \033[0m.
The three tests/unit/test_utils.py assertions follow from the change.

did/stats.py

The failure path logged Skipping %s due to %s with a
concurrent.futures.Future as the first argument, so it rendered a repr:

Skipping <Future at 0x1063a4830 state=finished raised ReportError> due to Unable to fetch token

This change keeps only the last part, error text.

🤖 Generated with Claude Code

Two unrelated-to-each-other-but-both-cosmetic fixes to how errors reach
the user, split out of psss#469 where they did not belong.

did/utils.py: the reset sequence emitted by color() was "\033[1;m", two
SGR parameters where the second is empty. Per ECMA-48 an empty parameter
defaults to 0, so a conforming terminal does end up reset, and this is
not a bug on one. But it relies on that defaulting rule, which parsers
that are not full terminals (log scrapers, CI log viewers, less without
-R, some ANSI-stripping libraries) handle inconsistently, and it briefly
enables bold on the way to resetting. "\033[0m" is the canonical reset
and does not read as a typo. The three test_utils.py assertions follow
from the change.

did/stats.py: the failure path logged "Skipping %s due to %s" with a
concurrent.futures.Future as the first argument, so it rendered a repr:

    Skipping <Future at 0x1063a4830 state=finished raised ReportError>
    due to Unable to fetch token

The address and state tell the user nothing, and the one useful piece of
information (which stat failed) is not there at all, since Future has no
back-reference to the submitted callable. Dropping the useless half
leaves the error text. Naming the stat properly would need a
future -> stat mapping, which is a larger change than this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ammachado
ammachado marked this pull request as ready for review September 7, 2026 18:37
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