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/01/17 15:56:05 UTC

[GitHub] [incubator-nuttx] patacongo opened a new issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

patacongo opened a new issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120
 
 
   When ran against any file that has comments to the right of definitions, a false alarm can occur.  Such right-hand comments should be vertically aligned, but have not specific alignment requirement with regard to the start of the line or any indentation.
   
   In many casese, there are comments on a separate line are correctly aligned with the other right hand comments.  This is necessary for the visually alignment, but results in a false alarm error from nxstyle.  Consider the following from include/nuttx/can/can.h:
   
       579   struct can_txfifo_s  cd_xmit;          /* Describes transmit FIFO */
       580 #ifdef CONFIG_CAN_TXREADY
       581   struct work_s        cd_work;          /* Use to manage can_txready() work */
       582 #endif
       583                                          /* List of pending RTR requests */
       584   struct can_rtrwait_s cd_rtr[CONFIG_CAN_NPENDINGRTR];
       585   FAR const struct can_ops_s *cd_ops;    /* Arch-specific operations */
   
   Running nxstyle against that file generates this false alarm:
   
       include/nuttx/can/can.h:583:1: error: Missing blank line after comment

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] johannes-nivus commented on issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

Posted by GitBox <gi...@apache.org>.
johannes-nivus commented on issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120#issuecomment-596241649
 
 
   @patacongo I had again a little fun with nxstyle this afternoon.
   I addressed Issue #120 and the problem had been quite simple: The prevbrhcmt had to propagate over preprocessor lines.
   Additionally I added checks for the column position of the right of code comments. (And caught one in include/nuttx/can/can.h)
   I don't want to file a PR without review (I fear your instant merging powers). So please take a look at
   https://github.com/johannes-nivus/incubator-nuttx/tree/nxstyle and review.
   Regards, Johannes  

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120#issuecomment-596244281
 
 
   The tests that should be performed are:
   
   1. Using a test case, verify that positirves are detected.
   
   2. Check for false negatives/positive regression.  I do that with a little script something like:
   
       #!/bin/sh
       
       FILELIST=`find sched -name *.c`
       for file in $FILELIST; do
           tools/nxstyle $file
       done
   
   Then run that before the change like:
   
       ./doit.sh 1>before.txt 2>&1
   
   Then rebuild with the modified nxstyle.c and:
   
       ../doit.sh 1>after.txt 2>&1
   
   Then compare before.txt with after .txt.  They should be identical or, if there are false positives/negatives in before.txt, then after.txt might have fixed those.
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120#issuecomment-596242782
 
 
   I never look at code outside of the respository.  Not interested.
   
   PS:  Everyone on the Apache time as instant merging power and use that power frequently.

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo closed issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

Posted by GitBox <gi...@apache.org>.
patacongo closed issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120
 
 
   

----------------------------------------------------------------
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


With regards,
Apache Git Services

[GitHub] [incubator-nuttx] patacongo commented on issue #120: tools/nxstyle.c: FALSE alarms on right hand aligned comments.

Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #120: tools/nxstyle.c:  FALSE alarms on right hand aligned comments.
URL: https://github.com/apache/incubator-nuttx/issues/120#issuecomment-596226398
 
 
   I think that the problem is an ordering one in nxstyle.c:
   
   - prevbrhcmt is true if the previous line is a right hand comment (brhcomment)
   - It is set at line 661 if the last line was a right hand comment
   - The error occurs because prevbrhcmt is not set at line 718 or 
   - Line 963 and 1395:  right hand comment should propagate to a single line comment if single line comment was preceded by  a right hand comment with no blank line.
   
   I think that the error occurs because the test of prevbrhcmt at line 718 occurs BEFORE it is set the the current line at lines 963 or 1395
   

----------------------------------------------------------------
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


With regards,
Apache Git Services