-
Notifications
You must be signed in to change notification settings - Fork 123
Clean up error output: color reset sequence and stat failure message - #482
Open
ammachado wants to merge 1 commit into
Open
Clean up error output: color reset sequence and stat failure message #482ammachado wants to merge 1 commit into
ammachado wants to merge 1 commit into
Conversation
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>
4 tasks
kwk
kwk
approved these changes
Sep 7, 2026
ammachado
marked this pull request as ready for review
September 7, 2026 18:37
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two fixes
did/utils.pyThe reset sequence emitted by
color()was033円[1;m, where it should be033円[0m.The three
tests/unit/test_utils.pyassertions follow from the change.did/stats.pyThe failure path logged
Skipping %s due to %swith aconcurrent.futures.Futureas the first argument, so it rendered a repr:This change keeps only the last part, error text.
🤖 Generated with Claude Code