-
Notifications
You must be signed in to change notification settings - Fork 1.6k
re-worded some error messages and avoid using product name #8074
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1224,7 +1224,7 @@ std::string MathLib::calculate(const std::string &first, const std::string &seco | |
| return MathLib::toString(MathLib::toBigNumber(first) ^ MathLib::toBigNumber(second)) + intsuffix(first, second); | ||
|
|
||
| default: | ||
| throw InternalError(nullptr, std::string("Unexpected action '") + action + "' in MathLib::calculate(). Please report this to Cppcheck developers."); | ||
| throw InternalError(nullptr, std::string("Unexpected action '") + action + "' in MathLib::calculate(). Please report this to the developers."); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not so sure about explicit mentions to report this. I would hope that users are reporting any of the internal errors they cannot fix by themselves.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes this can be discussed. I don't see many such reports either because people don't report or because they don't see this. Personally, I rarelly report bugs I see in software.
this sentence could be interpreted as we are saying that the developers of the checked code should fix this themselves.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Right - this made it ambiguous. I will just drop it as a whole. |
||
| } | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -959,7 +959,7 @@ void Preprocessor::missingInclude(const std::string &filename, unsigned int line | |
| } | ||
| ErrorMessage errmsg(std::move(locationList), mFile0, Severity::information, | ||
| (headerType==SystemHeader) ? | ||
| "Include file: <" + header + "> not found. Please note: Cppcheck does not need standard library headers to get proper results." : | ||
| "Include file: <" + header + "> not found. Please note: Standard library headers do not need to be provided to get proper results." : | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aside from dropping the product name I think this explains it better.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. lgtm |
||
| "Include file: \"" + header + "\" not found.", | ||
| (headerType==SystemHeader) ? "missingIncludeSystem" : "missingInclude", | ||
| Certainty::normal); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3152,7 +3152,7 @@ bool TemplateSimplifier::simplifyTemplateInstantiations( | |
| "templateRecursion", | ||
| "TemplateSimplifier: max template recursion (" | ||
| + std::to_string(mSettings.maxTemplateRecursion) | ||
| + ") reached for template '"+typeForNewName+"'. You might want to limit Cppcheck recursion.", | ||
| + ") reached for template '"+typeForNewName+"'. You might want to adjust the template recursion limit.", | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if we should mention that the limit should be adjusted.
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. imho I prefer to mention that it CAN be adjusted not that it SHOULD be adjusted. This change lgtm.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should probably drop that sentence as a whole. If you reference to a knob we might need to mention it but then that would be a CLI option and be of no value for GUI user (thinking of that we should never mention any option in the messages - but something for later). |
||
| Certainty::normal); | ||
| mErrorLogger.reportErr(errmsg); | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
Aside from dropping the product name I think this explains it better.
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.
ok. I have no strong opinion.
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.
It still feels a bit awkward but at least it is a bit more specific and the product name is gone.