Skip to content

refactor: clarify printf-like message to exclude all format specifiers - #96

Open
scop wants to merge 1 commit into
timonwong:masterfrom
scop:refactor/printf-message
Open

refactor: clarify printf-like message to exclude all format specifiers#96
scop wants to merge 1 commit into
timonwong:masterfrom
scop:refactor/printf-message

Conversation

@scop

@scop scop commented Nov 6, 2024

Copy link
Copy Markdown

The previous wording can raise questions and false assumptions that some format specifiers would be ok, but none actually are.

@scop
scop force-pushed the refactor/printf-message branch from 07cd458 to 7c99273 Compare May 28, 2025 13:44
The previous wording can raise questions and false assumptions that
_some_ format specifiers would be ok, but none actually are.
@scop
scop force-pushed the refactor/printf-message branch from 7c99273 to 36cbd64 Compare May 28, 2025 13:50

log.Info("%[3]*s x") // want `logging message should not use format specifier ".+"`
log.Info("%[3]d x") // want `logging message should not use format specifier ".+"`
log.Info("%[3]*s x") // want `logging message should not contain format specifiers, found ".+"`

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

My comment is not on what you changed, but on what was reported here

%[3]s leads to .+ ?

It's something that come from the external lib code imported from golang.org/x/tool but it's unclear to me what it could mean.

Do you?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I guess but have not verified at all, that the line could have been written as this instead:

	log.Info("%[3]*s x") // want `logging message should not contain format specifiers, found "%\[3\]\*s"`

So I guess the "want" comment "argument" strings in backticks are actually regular expressions, and because the specifier %[3]*s contains a few regular expression metacharacters, they would need escaping if it was written literally.

Instead, matching it with simply .+ could have been seen good enough by the original author. Or maybe there was some problem with metacharacter escaping, I don't know (nor do I know if a simple backslash would do the trick or if something else would be required).

@codecov

codecov Bot commented May 30, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.79%. Comparing base (8ad5446) to head (36cbd64).

Additional details and impacted files
@@           Coverage Diff           @@
##           master      #96   +/-   ##
=======================================
  Coverage   98.79%   98.79%           
=======================================
  Files          13       13           
  Lines         582      582           
=======================================
  Hits          575      575           
  Misses          5        5           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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