New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Change: [strgen] 'warnings' for translations are now 'infos' unless invoked with -w or -t #9406
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This commit message is way to specific :P It is okay to use extra lines in the commit message, like:
Change: [strgen] Change warnings for translations into infos
Unless invoked with -w ("show warnings") or -t ("show todos")
Eints normally fixes the warnings after a Pull Request, so it is not
really useful information for the developer to see as a warning.
At least, I am guessing that is what -w and -t do, please feel free to correct where needed :P
fprintf(stderr, LINE_NUM_FMT("warning"), _file, _cur_line, buf); | ||
} else { | ||
fprintf(stderr, LINE_NUM_FMT("info"), _file, _cur_line, buf); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why would the classification change depending on a setting? I am not sure I follow that logic. For who will it become a warning that doesn't think info is what he is looking for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure to fully understand your question.
-t
sets bit 0 of _show_todo
, -w
sets bit 1, and now english.txt sets bit 2.
So if any bit is set it's a warning, else it's an info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I am wondering why this if-statement exists in the first place. Why would it be a "warning" in some cases, and "info" in others. That always feels a bit weird, and often tells there is something else being a bit off. I haven't checked how other places do it. I just don't get why the log-level would change depending on -t
for example.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, a bit closer look, I do not think this is the right approach. There are plenty of warnings that really are warnings, and never infos. Like invalid UTF-8, for example. What if we just add a new command like strgen_translation_warning
, in that case I could understand this if-statement a lot more :)
Edit: or better yet, just don't output anything if -w
and -t
aren't set for translation errors? :D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nevermind all this, it is already messy as fuck. So it is not fair to ask from you to fix "everything" just to fix a bug :P So lets just do this.
Unless invoked with -w, --warning ("print a warning for any untranslated strings") or -t, --todo ("replace any untranslated strings with '<TODO>'"). Eints normally fixes the warnings after a Pull Request, so it is not really useful information for the developer to see as a warning.
Motivation / Problem
A change in gcc-problem-matcher made it pickup strgen warnings for translations. These are usually handled by eints later.
Description
Use
info
for translation warnings, unless-w
or-t
switches are used.This is done by abusing
_show_todo
.Tested on #9017 (see output here).
I also tested locally to be sure
english.txt
still always generatewarning
and notinfo
.Limitations
Checklist for review
Some things are not automated, and forgotten often. This list is a reminder for the reviewers.