You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/05/19 22:23:33 UTC
[GitHub] [incubator-nuttx] patacongo opened a new pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
patacongo opened a new pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079
## Summary
For some unknow reason, long lines were reported as WARNING is header files, but as ERRORS in C source file. There are errors where ever they occur.
## Impact
None other than a visual difference. All long lines will be reportes as ERRORs.
## Testing
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631150124
Can one of you please merge this so that we can get on with things.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631475780
> > We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
>
> I am not sure why you say artificially. I think it looks better, but the lines may be longer.
Another thing that you can do to keep the comment alignment from propogating like this is to simply insert a blank line. The right hand comments AFTER the blank line do not have to align to the same column as the right hand comends before the blank line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631433245
> What is the thinking on this: ...
>
> We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
I am not sure why you say artificially. I think it looks better and is more readable, but the lines may be longer which could be problem in some cases.
> Was this always coding standards violation or the defacto format?
Always. The coding standard says that right hand comments must be vertically aligned (https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#comments). nxstyle now enforces that requirement since a series of commits by Johanne Schock around Mar. 8
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan merged pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
hartmannathan merged pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631144775
> The reason was this:
>
> In past conversation you had told me that readability was more important than line length and that long lines were OK in arch headers. The code base reflected that.
I have no recollection of that in the past. But we cut a lot of corners in the past that we no longer do.
> It is sad to me that the code has become less readable and information has been removed to accommodate the line length. I took pride in following your tradition of reflecting the datasheets in the headers.
This I disagree with 1000%. The headers should not duplicate all of the information in the datasheets. That is just wrong. Comments should simply identify the field. Look in the data sheet if you want a long detailed description. Don't force it to the right side of a #define. That is hideous.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631406180
@patacongo Thank you for sorting this out!
What is the thinking on this:
We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
Was this always coding standards violation or the defacto format?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] davids5 commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631135587
> For some unknow reason, long lines were reported as WARNING is header files, but as ERRORS in C source file.
The reason was this:
In past conversation you had told me that readability was more important than line length and that long lines were OK in arch headers. The code base reflected that.
But it is hard to teach a tool what any of that that means.
It is sad to me that the code has become less readable and information has been removed to accommodate the line length. I took pride in following your tradition of reflecting the datasheets in the headers.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631147120
> Readability **_is_** important
BUT a header file is not a substitute for the data sheet. It should contain only a terse statement sufficient for you to find the detailed information in the data sheet or reference manual. This error that has been made that got us into to this position is simply putting too much shit in comments.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631433245
> What is the thinking on this: ...
>
> We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
I am not sure why you say artificially. I think it looks better and is more readable, but the lines may be longer which could be problem in some cases.
> Was this always coding standards violation or the defacto format?
Always. The coding standard says that right hand comments must be vertically algined (https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#comments). nxstyle now enforces that requirement since a series of commits by Johanne Schock around Mar. 8
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631156904
> @hartmannathan I am sure you noticed the typo "be aligned so that the comment begins in the same _comment_ on each line." Of course that should have been _column_. I bet that made you cringe.
Yes. :-)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631401833
> > In past conversation you had told me that readability was more important than line length and that long lines were OK in arch headers. The code base reflected that.
>
> I have no recollection of that in the past. But we cut a lot of corners in the past that we no longer do.
@davids5 I think I know what this is about. Header files do typically have longer lines than C source files. But that doesn't make excessively long lines more acceptable in header files.
There have been changes in nxstyle from several months ago that you may not be aware of. The old nxstyle used a fixed, default line width of 78 (unless you overrode that with the -m option). But that logic has been relaced. It now determines the line width dynamically using the width of the block comment "rulers."
So a long line error means that a line has extended beyond the rulers. That should be reported as an error regardless of the file type.
I don't think this effects the readability of the header files unless people cram too much garbage in the right hand comments. Then the readability is bad in any case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631165597
I also fixed the coding standard problems that I noted above with PR #1083
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631433245
> What is the thinking on this: ...
>
> We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
I am not sure why you say artificially. I think it looks better, but the lines may be longer.
> Was this always coding standards violation or the defacto format?
Always. The coding standard says that right hand comments must be vertically algined (https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#comments). nxstyle now enforces that requirement since a series of commits by Johanne Schock around Mar. 8
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631475780
> > We are artificially making the file wider because the tool does not parse the h files as they were and have always been.
>
> I am not sure why you say artificially. I think it looks better, but the lines may be longer.
Another thing that you can do to keep the comment alignment from propogating like this is to simply insert a blank line. The right hand comments AFTER the blank line to no have to align to the same column as the right hand comends before the blank line.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631144775
> The reason was this:
>
> In past conversation you had told me that readability was more important than line length and that long lines were OK in arch headers. The code base reflected that.
I have no recollection of that in the past. But we cut a lot of corners in the past that we no longer do.
> It is sad to me that the code has become less readable and information has been removed to accommodate the line length. I took pride in following your tradition of reflecting the datasheets in the headers.
This I disagree with 1000%. The headers should not duplicate all of the information in the datasheets. That is just wrong. Comments should simply identify the field. Look in the data sheet if you want a long detailed description. Don't force it to the right side of a #define. That is hideous.
That was never a tradition. The tradition is the coding standard. I quote: "Comments to the Right of Statements. Comments to the right of statements in C source files are _discouraged_. If such comments are used, they should be (1) _very short so that they do not exceed the line width_ (typically 78 characters), (2) fit on one line, and (3) be aligned so that the comment begins in the same comment on each line." https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#comments
That is the tradition. That is the standard for right hand comments that has existed for 10 years or so. One thing we don't follow now is _fit on one line_. There are plenty of long multi-line comments to the right in the code base.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] hartmannathan commented on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631138388
> It is sad to me that the code has become less readable and information has been removed to accommodate the line length. I took pride in following your tradition of reflecting the datasheets in the headers.
I think that can still be done: In earlier conversations, it was agreed that nxstyle would decide what is a "long" line by looking at the length of `/***********` at the top of the file. So, the solution for arch headers is to have a longer header, to allow for readability while also having important register/bit information on the same line as the defines.
Readability **_is_** important, so I hope the above is correct.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo edited a comment on pull request #1079: nxstyle: Fix distinction be source and header file for long lines.
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on pull request #1079:
URL: https://github.com/apache/incubator-nuttx/pull/1079#issuecomment-631144775
> The reason was this:
>
> In past conversation you had told me that readability was more important than line length and that long lines were OK in arch headers. The code base reflected that.
I have no recollection of that in the past. But we cut a lot of corners in the past that we no longer do.
> It is sad to me that the code has become less readable and information has been removed to accommodate the line length. I took pride in following your tradition of reflecting the datasheets in the headers.
This I disagree with 1000%. The headers should not duplicate all of the information in the datasheets. That is just wrong. Comments should simply identify the field. Look in the data sheet if you want a long detailed description. Don't force it to the right side of a #define. That is hideous.
That was never a tradition. The tradition is the coding standard. I quote: "Comments to the Right of Statements. Comments to the right of statements in C source files are _discouraged_. If such comments are used, they should be (1) _very short so that they do not exceed the line width_ (typically 78 characters), (2) fit on one line, and (3) be aligned so that the comment begins in the same comment on each line." https://cwiki.apache.org/confluence/display/NUTTX/Coding+Standard#comments
That is the tradition. That is the standard for right hand comments that has existed for 10 years or so. One thing we don't follow now is _fit on one line_. There are plenty of long multi-line comments to the right in the code base.
@hartmannathan I am sure you noticed the typo "be aligned so that the comment begins in the same _comment_ on each line." Of course that should have been _column_. I bet that made you cringe.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org