-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
possible nxstyle false alarm: "Missing space before closing C comment" #540
Comments
Change to?
|
@xiaoxiang781216 |
I think it isn't a false alarm, it shows there is something wrong: |
I don't read the style guiding recently, not sure whether the guide require we have only three possible location to put the comment. My suggestion just try to make nxsytle happy:). |
The coding standard is clear about any form of commented out code. It should be avoided unless there is explanation, then the may be disabled only with #if 0, never commented out. |
The discussion isn't about commented out code, but about comments inside code, perhaps parameter lists:
|
Yes, but this case is violating the coding standard for other reasons and is fixed with #542 Please merge, someone. That is gating other PRs. |
This is the error: At line 1474: line[n-1] == '*' and line[n] == '/' and n >= 2 This test at line 1474 is line[n+1] != ' ' and line[n-2] != '*' That should be line[n-1] != ' ', right? (actually !isspace(line[n-1])). It is trying to verify that */ is preceded by a space like " */" or by another asterisk as in a block comment "**/" This is just a dumb coding error. Do you want to fix it? Or should I? Let's not collide. |
I see, but can you please share your opinion concerning comments inside statements that is not commented out code but perhaps an explanation?
|
If it is not prohibited, then it is permitted. Providing it does not conflict with common sense, acceptable practice. And if it conflicts with common sense, acceptable practice then it should be prohibited. I don't have a strong opinion in this case. I would not comment in that way[1]. And would raise my eybrows to see such commenting. Ultimately coding standards come down to aesthetics. But the coding standard must be commonly applied for good software quality. So those aesthetics need to be formalized and made standard .. cast in concrete. If they are not so formalized, then it is left as a matter of personal preference UNLESS you do want to formalilze it. If you want to forbid comments in the argument lists, I will support that but then you should also submit a PR against Documentation/NuttXCCodingStandard.html. That is the master coding standard, the version on the website is a copy. Greg [1] CAVEAT: I don't normally comment this way, but I am the culprit in the example we were discussing. In this case, I just left some commented out stuff left over from debug. I still think that the commented out code is correct but it is commented out because it didn't work and I forgot to remove the comments. In #542 I kept the question but moved it out of the argument list and marked it clearly with REVISIT: |
The issue here is just a nxstyle bug at line 1474. But thinking more about the larger question, "Should comments be allowed in parameter lists?" Ithink that the answer has to be YES. After reconsideration I realize that that is actually done quite often. The example case at hand is perverse, but parameter commenting like the following is very common and should be permitted (in both actual and formal parameters):
I have done that a few times myself when the values passed as actual parameters are not clear. It is probably not a good thing to do for formal parameters since these should be documented in the "Input Parameters" section of the function header, not in the parameter list. However, many people do put comments on formal parameters. |
A slightly more credible example:
In mathematical logic, sometimes the arguments can be relatively complex expressions mat not be intuitive and should be commented. In the above case, I would probably use local variables to hold at least the height and width, but others do things differently. You get the general idea. |
I will fix it, give me one or two hours. Concerning the comment stuff, I get the idea, and currently your examples shouldn't produce problems... but I will test. |
…mment was faulty
Closing... This should be fixed with #543 |
apache#543) *Addresses Issue apache#540, check for missing space before closing comment was faulty
nxstyle claims "Missing space before closing C comment" on lines like the following:
in sched/sched/sched_mergepending.c.
i don't understand what it isn't happy about.
The text was updated successfully, but these errors were encountered: