You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@nuttx.apache.org by GitBox <gi...@apache.org> on 2019/12/27 21:21:06 UTC
[GitHub] [incubator-nuttx] davids5 opened a new pull request #12: nxstyle
improvements with No tooling
davids5 opened a new pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12
- Added features
- outputs parse-able compiler like error format
- Uses getops to parse command line.
- Supports
-s silence all output
-g provide a PASS fail message
test script to check the whole of nuttx
```
#!/bin/bash
LIST=$(find ../ -type f \( -iname \*.c -o -iname \*.h \))
for f in $LIST; do
./nxstyle -g -m 80 $f;
done
```
Good new 4000 files pass.
```
./test.sh 2>&1 | grep PASSED | wc -l
4000
```
BAD new 5298 files fail.
```
./test.sh 2>&1 | grep FAIL | wc -l
5298
```
# Test cases
## Test case 1
ironic but true
`./nxstyle -m 99 nxstyle-master.c `
```
nxstyle-master.c:316:11: error: Bad alignment
nxstyle-master.c:319:13: error: Bad left brace alignment
nxstyle-master.c:320:15: error: Bad comment alignment
nxstyle-master.c:321:16: error: Bad comment block alignment
nxstyle-master.c:322:16: error: Bad comment block alignment
nxstyle-master.c:323:16: error: Bad comment block alignment
nxstyle-master.c:325:15: error: Bad alignment
nxstyle-master.c:329:17: error: Bad left brace alignment
nxstyle-master.c:333:17: error: Bad right brace alignment
nxstyle-master.c:335:15: error: Bad comment alignment
nxstyle-master.c:336:16: error: Bad comment block alignment
nxstyle-master.c:337:16: error: Bad comment block alignment
nxstyle-master.c:338:16: error: Bad comment block alignment
nxstyle-master.c:339:16: error: Bad comment block alignment
nxstyle-master.c:341:15: error: Bad alignment
nxstyle-master.c:342:17: error: Bad left brace alignment
nxstyle-master.c:345:19: error: Bad alignment
nxstyle-master.c:346:21: error: Bad left brace alignment
nxstyle-master.c:347:21: error: Bad right brace alignment
nxstyle-master.c:349:19: error: Bad alignment
nxstyle-master.c:351:21: error: Bad left brace alignment
nxstyle-master.c:353:21: error: Bad right brace alignment
nxstyle-master.c:354:17: error: Bad right brace alignment
nxstyle-master.c:355:13: error: Bad right brace alignment
nxstyle-master.c:406:1: error: Missing blank line after comment
nxstyle-master.c:480:10: error: C++ style comment
nxstyle-master.c:540:1: error: Missing blank line after comment
nxstyle-master.c:609:0: error: C++ style comment
nxstyle-master.c:609:0: error: No indentation line
nxstyle-master.c:611:0: error: C++ style comment
nxstyle-master.c:611:0: error: No indentation line
nxstyle-master.c:617:0: error: C++ style comment
nxstyle-master.c:617:0: error: No indentation line
nxstyle-master.c:891:100: error: Long line found
nxstyle-master.c:1189:1: error: Too many blank lines
nxstyle-master.c:1211:56: error: Missing space before closing C comment
nxstyle-master.c:1330:1: error: Missing blank line before comment found
nxstyle-master.c:1356:1: error: Missing blank line before comment found
nxstyle-master.c:1405:1: error: Missing blank line before comment found
nxstyle-master.c:1446:1: error: Missing blank line before comment found
nxstyle-master.c:1474:105: error: Long line found
nxstyle-master.c:1501:1: error: Missing blank line before comment found
nxstyle-master.c:1516:1: error: Missing blank line before comment found
nxstyle-master.c:1544:1: error: Missing blank line before comment found
nxstyle-master.c:1572:1: error: Missing blank line before comment found
nxstyle-master.c:1592:1: error: Missing blank line before comment found
nxstyle-master.c:1607:1: error: Missing blank line before comment found
nxstyle-master.c:1626:1: error: Missing blank line before comment found
nxstyle-master.c:1744:38: error: Operator/assignment must be preceded with whitespace
```
## Test case 2
blank.c
`nxstyle -m 90 blank.c`
45 lines of /n
```
...
```
` ./nxstyle -m 80 blank.c `
```
blank.c:1:1: error: File begins with a blank line
blank.c:2:1: error: Too many blank lines
blank.c:3:1: error: Too many blank lines
blank.c:4:1: error: Too many blank lines
blank.c:5:1: error: Too many blank lines
blank.c:6:1: error: Too many blank lines
blank.c:7:1: error: Too many blank lines
blank.c:8:1: error: Too many blank lines
blank.c:9:1: error: Too many blank lines
blank.c:10:1: error: Too many blank lines
blank.c:11:1: error: Too many blank lines
blank.c:12:1: error: Too many blank lines
blank.c:13:1: error: Too many blank lines
blank.c:14:1: error: Too many blank lines
blank.c:15:1: error: Too many blank lines
blank.c:16:1: error: Too many blank lines
blank.c:17:1: error: Too many blank lines
blank.c:18:1: error: Too many blank lines
blank.c:19:1: error: Too many blank lines
blank.c:20:1: error: Too many blank lines
blank.c:21:1: error: Too many blank lines
blank.c:22:1: error: Too many blank lines
blank.c:23:1: error: Too many blank lines
blank.c:24:1: error: Too many blank lines
blank.c:25:1: error: Too many blank lines
blank.c:26:1: error: Too many blank lines
blank.c:27:1: error: Too many blank lines
blank.c:28:1: error: Too many blank lines
blank.c:29:1: error: Too many blank lines
blank.c:30:1: error: Too many blank lines
blank.c:31:1: error: Too many blank lines
blank.c:32:1: error: Too many blank lines
blank.c:33:1: error: Too many blank lines
blank.c:34:1: error: Too many blank lines
blank.c:35:1: error: Too many blank lines
blank.c:36:1: error: Too many blank lines
blank.c:37:1: error: Too many blank lines
blank.c:38:1: error: Too many blank lines
blank.c:39:1: error: Too many blank lines
blank.c:40:1: error: Too many blank lines
blank.c:41:1: error: Too many blank lines
blank.c:42:1: error: Too many blank lines
blank.c:43:1: error: Too many blank lines
blank.c:44:1: error: Too many blank lines
blank.c:45:1: error: Too many blank lines
blank.c:45:1: fatal: "Private/Public Functions" not found! File was not be checked
```
## Test case 3
```
./nxstyle -m 99 nxstyle.c`
```
## Test case 4
```
./nxstyle -g -m 99 nxstyle.c
nxstyle.c: PASSED nxstyle check
```
## Test case 5
```
./nxstyle -g -m 99 nxstyle.c; echo $?
nxstyle.c: PASSED nxstyle check
0
```
## Test case 6
```
./nxstyle -g -m 99 nxstyle-master.c; echo $?
nxstyle-master.c: FAILED nxstyle check
1
```
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422728
The use of '-m 99' in the testing is a little odd. I usuaully use -m 86. This probably raises some tool use questions. The coding standard says:
Line Width. Text should not extend past column 78 in the typical C source or header file. Sometimes the nature of the content of a file may require that the lines exceed this limit. This often occurs in header files with naturally long definitions. If the line width must extend 78 lines, then some wider line width may be used in the file provided that it is used consistently.
This should match the width of the block comments:
Block Comments. Each grouping in the file is separated with a block comment. The block comment consists of:
- A line that consists of the opening C comment (/*) followed by a series of asterisks extending to the length of the line (usually to column 78).
- The name of the grouping, starting at column 4. An asterisk preceives the name of the grouping in column 1.
- A line that consists of the closing C comment (*/) at the end of the line (usually column 78) preceded by a series of asterisks extending to column 1.
So picking any fixed value will cause long lines to be missed OR many invalid "long line" error reports. Ideally, we should parse the file first, look at the block comments, make sure that they are the same width, and set the maximum line width automatically.
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569671947
@xiaoxiang781216 - all review Items have been addressed - Please merge or ask an other PPMC to merge it.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361989296
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
Review comment:
1.remove filename assignment: not valid, and dup with 275
2.change maxline to g_maxline? let's make all command line option to global for consistence
3.Can we merge g_gonogo and silient into one(e.g. g_verbose)?
g_verbose == 0, no output
g_verbose == 1, PASS/FAIL
g_verbose == 2, each fail line
4.Move the assignment to variable definition:
static int g_verbose = 2;
static int g_maxline = 78;
....
----------------------------------------------------------------
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] davids5 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569500865
# How to use nxstyle with **style**
Eclipse with CDT (https://gnu-mcu-eclipse.github.io/) Empowered development with SVD enabled debugging.
## Setup Tab "Builder Setting"
## Setup Tab "Behavior"
## Tool in action
- select a file (open it)
- Select nxstyle as the tool
- The Clck the Hammer!
## Get a list of CS violations
## Click and fix them! (hit work bottom up)
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361991442
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
+ g_file_name = filename;
instream = fopen(filename, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ g_gonogo = false;
+ FATALFL("Failed to open", filename);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ hdrfile = false;
+ g_file_type = UNKNOWN;
+ ext = strrchr(filename, '.');
if (ext == 0)
{
- printf("No file extension\n");
+ WARN("No file extension", 0 , 0);
}
else if (strcmp(ext, ".h") == 0)
{
hdrfile = true;
+ g_file_type = C_HEADER;
}
- else if (strcmp(ext, ".c") != 0)
+ else if (strcmp(ext, ".c") == 0)
+ {
+ g_file_type = C_SOURCE;
+ }
+ else
{
printf("Unrecognized file extension: \"%s\"\n", ext + 1);
Review comment:
How about we silient the unknow file? so we can integrate with the workflow more easier?
----------------------------------------------------------------
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 merged pull request #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo merged pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569424363
@patacongo - would you please build and run this under windows.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362011203
##########
File path: tools/nxstyle.c
##########
@@ -134,70 +211,80 @@ int main(int argc, char **argv, char **envp)
int rbrace_lineno; /* Last line containing a right brace */
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
- int maxline; /* Lines longer that this generate warnings */
int n;
int i;
+ int c;
- maxline = 78;
- filename = argv[1];
-
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ while ((c = getopt(argc, argv, ":hv:gm:")) != -1)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
- }
- else if (argc == 2)
- {
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ g_maxline = atoi(optarg);
+ if (g_maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 'v':
+ g_verbose = atoi(optarg);
+ if (g_verbose < 0 || g_verbose > 2)
+ {
+ show_usage(argv[0], 1, "Bad value for <level>.");
+ }
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
- instream = fopen(filename, "r");
+ g_file_name = argv[optind];
+ instream = fopen(g_file_name, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ FATALFL("Failed to open", g_file_name);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ hdrfile = false;
+ g_file_type = UNKNOWN;
+ ext = strrchr(g_file_name, '.');
if (ext == 0)
{
- printf("No file extension\n");
+ WARN("No file extension", 0 , 0);
}
else if (strcmp(ext, ".h") == 0)
{
hdrfile = true;
Review comment:
should we remove hdrfile use g_file_type == C_HEADER instead?
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361988441
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
Review comment:
remove if (...){...}, the check at line 270 is enough
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569467729
It is not clear the the disinction between warning, error, and fatal makes any sense for coding standby. isn't is a boolean; The code complies to the standard or it does not. One non-compliance is no better or worse than another.
But I will leave that to Xiao Xiang and Haitao Liu to determine if they believe that that concept is somehow useful.
How dif you decide what is an warning, or an error, or fatal. Do you have some criteria for degree of meeting the coding standard?
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569731061
@xiaoxiang781216
The program needs many more functional changes to be useful, My vacation is coming to an end.
I will not be investing any more time on this.
Please feel free to have your team make any further changes like adding var args it xiaomi needs them.
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569467729
It is not clear the the disinction between warning, error, and fatal makes any sense for coding standby. isn't is a boolean; The code complies to the standard or it does not. One non-compliance is no better or worse than another.
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569447104
> Just noticed... tools/README.txt also needs to be updated. I think just the old Usage comments need to be replaced with the new Usage comments.
@patacongo - Thank you for noticing. It is added.
Did it run ok on Windows?
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007291
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
Review comment:
Done!
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569706666
@xiaoxiang781216 - thank you for a through review and all the value added suggestions.
I am done, all the changes: I added a version in the help. I suspect we will need this as the tool evolves.
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422728
The use of '-m 99' in the testing is a little odd. I usuaully use -m 86. This probably raises some tool use questions. The coding standard says:
Line Width. Text should not extend past column 78 in the typical C source or header file. Sometimes the nature of the content of a file may require that the lines exceed this limit. This often occurs in header files with naturally long definitions. If the line width must extend 78 lines, then some wider line width may be used in the file provided that it is used consistently.
This should match the width of the block comments:
Block Comments. Each grouping in the file is separated with a block comment. The block comment consists of:
A line that consists of the opening C comment (/*) followed by a series of asterisks extending to the length of the line (usually to column 78).
The name of the grouping, starting at column 4. An asterisk preceives the name of the grouping in column 1.
A line that consists of the closing C comment (*/) at the end of the line (usually column 78) preceded by a series of asterisks extending to column 1.
So picking any fixed value will cause long lines to be missed OR many invalid "long line" error reports. Ideally, we should parse the file first, look at the block comments, make sure that they are the same width, and set the maximum line with automatically.
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422025
I have already review this in its previous incarnation. So I give my approval nw. I don't know if we can approvals from the other people on the list.
There is no workflow governing tools in nuttx/tools or apps/tools. C tools should follow C coding standard and the committer should satisfy themselves that the changes are functional. These change does not seem to affect the coding standard verification logic, only the form of error reporting so it is low risk.
So I would think that if/when you, @xiaoxiang781216 (and Haitao), are satisfied with the change, I think you should go ahead with the merge.
When there are changes to nxstyle that do affect the detection of problems, there is some very careful testing should be done to (1) assure that the change detects the problem and (2) does not introduce false alarms. I can share how I do that when the time is right.
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007745
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
+ g_file_name = filename;
instream = fopen(filename, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ g_gonogo = false;
Review comment:
Done! ea8b41d8db
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569743134
@xiaoxiang781216 Perhaps you should merge this onto a development branch. Where we can work together to continue development. It is currently based on the maste branch. You can create an nxstyle branch and change the PR base branch per https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
We can then cooperate with the remaining changes requested, Haitao Liu's changes, and the things that I want to do.
NOTE: Another further changes to the development branch must also be made via PR, not by direct merges to the branch.
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007489
##########
File path: tools/nxstyle.c
##########
@@ -43,53 +43,128 @@
#include <string.h>
#include <strings.h>
#include <ctype.h>
+#include <unistd.h>
+#include <libgen.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define LINE_SIZE 512
+#define LINE_SIZE 512
+
+#define FATAL(m, l, o) message(FATAL, (m), (l), (o))
+#define FATALFL(m,s) message(FATAL, (m), -1, -1)
+#define WARN(m, l, o) message(WARN, (m), (l), (o))
+#define ERROR(m, l, o) message(ERROR, (m), (l), (o))
+#define INFO(m, l, o) message(INFO, (m), (l), (o))
+
+/****************************************************************************
+ * Private types
+ ****************************************************************************/
+
+enum class_e
+{
+ INFO,
+ WARN,
+ ERROR,
+ FATAL
+};
+
+const char *class_text[] =
+{
+ "info",
+ "warning",
+ "error",
+ "fatal"
+};
+
+enum file_e
+{
+ UNKNOWN,
+ C_HEADER,
+ C_SOURCE
+};
+
+/****************************************************************************
+ * Private data
+ ****************************************************************************/
+
+static bool g_gonogo = false;
+static int g_status = 0;
+static char *g_file_name = "";
+static enum file_e g_file_type = UNKNOWN;
/****************************************************************************
* Private Functions
****************************************************************************/
-static void show_usage(char *progname, int exitcode)
+static void show_usage(char *progname, int exitcode, char *what)
{
- fprintf(stderr, "Usage: %s [-m <maxline>] <filename>\n", progname);
- fprintf(stderr, " %s -h\n", progname);
+ if (what)
+ {
+ fprintf(stderr, "%s\n", what);
+ }
+
+ fprintf(stderr, "Usage: %s [-m <maxline>] [-s] [-g] <filename>\n", basename(progname));
+ fprintf(stderr, " %s -h this help\n", basename(progname));
+ fprintf(stderr, " %s -s silent\n", basename(progname));
+ fprintf(stderr, " %s -g go no go output only\n", basename(progname));
exit(exitcode);
}
+static int message(enum class_e class, const char *text, int lineno, int ndx)
+{
+ FILE *out = stdout;
+
+ if (class > INFO)
+ {
+ out = stderr;
+ g_status |= 1;
+ }
+
+ if (!g_gonogo)
+ {
+ if (lineno == -1 && ndx == -1)
+ {
+ fprintf(out, "%s:%s: %s\n", class_text[class], text, g_file_name);
+ }
+ else
+ {
+ fprintf(out, "%s:%d:%d: %s: %s\n", g_file_name, lineno, ndx,
+ class_text[class], text);
+ }
+ }
+
+ return g_status;
+}
+
static void check_spaces_left(char *line, int lineno, int ndx)
{
/* Unary operator should generally be preceded by a space but make also
* follow a left parenthesis at the beginning of a parenthetical list or
* expression or follow a right parentheses in the case of a cast.
*/
- if (ndx > 0 && line[ndx - 1] != ' ' && line[ndx - 1] != '(' && line[ndx - 1] != ')')
+ if (ndx > 0 && line[ndx - 1] != ' ' && line[ndx - 1] !=
+ '(' && line[ndx - 1] != ')')
{
Review comment:
Done!
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007559
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
+ g_file_name = filename;
instream = fopen(filename, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ g_gonogo = false;
+ FATALFL("Failed to open", filename);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ hdrfile = false;
+ g_file_type = UNKNOWN;
+ ext = strrchr(filename, '.');
if (ext == 0)
{
- printf("No file extension\n");
+ WARN("No file extension", 0 , 0);
}
else if (strcmp(ext, ".h") == 0)
{
hdrfile = true;
+ g_file_type = C_HEADER;
}
- else if (strcmp(ext, ".c") != 0)
+ else if (strcmp(ext, ".c") == 0)
+ {
+ g_file_type = C_SOURCE;
+ }
+ else
{
printf("Unrecognized file extension: \"%s\"\n", ext + 1);
Review comment:
Done!
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361979138
##########
File path: tools/nxstyle.c
##########
@@ -1896,24 +1962,32 @@ int main(int argc, char **argv, char **envp)
else if (indent == 1 || indent == 3)
{
- fprintf(stderr, "Small odd alignment at line %d:%d\n",
- lineno, indent);
+ ERROR("Small odd alignment", lineno, indent);
}
}
}
}
if (!bfunctions && !hdrfile)
{
- fprintf(stderr, "ERROR: \"Private/Public Functions\" not found!\n");
- fprintf(stderr, " File could not be checked.\n");
+ FATAL("\"Private/Public Functions\" not found!"
+ " File was not be checked", lineno, 1);
}
if (ncomment > 0 || bstring)
{
- fprintf(stderr, "ERROR: In a comment/string at end of file\n");
+ FATAL("Comment or string found at end of file", lineno, 1);
Review comment:
```suggestion
ERROR("Comment or string found at end of file", lineno, 1);
```
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569743134
@xiaoxiang781216 Perhaps you should merge this onto a features branch, perhaps called 'nxstyle'. There we can work together to continue development. It is currently based on the master branch. You can create an nxstyle branch and change the PR base branch per https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
We can then cooperate with the remaining changes requested, Haitao Liu's changes, and the things that I want to do.
NOTE: Another further changes to the development branch must also be made via PR, not by direct merges to the branch.
PS: I can help with any of this. But I think you are in charge of the disposition of the PR.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361990500
##########
File path: tools/nxstyle.c
##########
@@ -43,53 +43,128 @@
#include <string.h>
#include <strings.h>
#include <ctype.h>
+#include <unistd.h>
+#include <libgen.h>
/****************************************************************************
* Pre-processor Definitions
****************************************************************************/
-#define LINE_SIZE 512
+#define LINE_SIZE 512
+
+#define FATAL(m, l, o) message(FATAL, (m), (l), (o))
+#define FATALFL(m,s) message(FATAL, (m), -1, -1)
+#define WARN(m, l, o) message(WARN, (m), (l), (o))
+#define ERROR(m, l, o) message(ERROR, (m), (l), (o))
+#define INFO(m, l, o) message(INFO, (m), (l), (o))
+
+/****************************************************************************
+ * Private types
+ ****************************************************************************/
+
+enum class_e
+{
+ INFO,
+ WARN,
+ ERROR,
+ FATAL
+};
+
+const char *class_text[] =
+{
+ "info",
+ "warning",
+ "error",
+ "fatal"
+};
+
+enum file_e
+{
+ UNKNOWN,
+ C_HEADER,
+ C_SOURCE
+};
+
+/****************************************************************************
+ * Private data
+ ****************************************************************************/
+
+static bool g_gonogo = false;
+static int g_status = 0;
+static char *g_file_name = "";
+static enum file_e g_file_type = UNKNOWN;
/****************************************************************************
* Private Functions
****************************************************************************/
-static void show_usage(char *progname, int exitcode)
+static void show_usage(char *progname, int exitcode, char *what)
{
- fprintf(stderr, "Usage: %s [-m <maxline>] <filename>\n", progname);
- fprintf(stderr, " %s -h\n", progname);
+ if (what)
+ {
+ fprintf(stderr, "%s\n", what);
+ }
+
+ fprintf(stderr, "Usage: %s [-m <maxline>] [-s] [-g] <filename>\n", basename(progname));
+ fprintf(stderr, " %s -h this help\n", basename(progname));
+ fprintf(stderr, " %s -s silent\n", basename(progname));
+ fprintf(stderr, " %s -g go no go output only\n", basename(progname));
exit(exitcode);
}
+static int message(enum class_e class, const char *text, int lineno, int ndx)
+{
+ FILE *out = stdout;
+
+ if (class > INFO)
+ {
+ out = stderr;
+ g_status |= 1;
+ }
+
+ if (!g_gonogo)
+ {
+ if (lineno == -1 && ndx == -1)
+ {
+ fprintf(out, "%s:%s: %s\n", class_text[class], text, g_file_name);
+ }
+ else
+ {
+ fprintf(out, "%s:%d:%d: %s: %s\n", g_file_name, lineno, ndx,
+ class_text[class], text);
+ }
+ }
+
+ return g_status;
+}
+
static void check_spaces_left(char *line, int lineno, int ndx)
{
/* Unary operator should generally be preceded by a space but make also
* follow a left parenthesis at the beginning of a parenthetical list or
* expression or follow a right parentheses in the case of a cast.
*/
- if (ndx > 0 && line[ndx - 1] != ' ' && line[ndx - 1] != '(' && line[ndx - 1] != ')')
+ if (ndx > 0 && line[ndx - 1] != ' ' && line[ndx - 1] !=
+ '(' && line[ndx - 1] != ')')
{
Review comment:
can we change to: if (ndx-- >0 && line[ndx] != ' ' && line[ndx] != '(' && line[ndx] != ')')?
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362010142
##########
File path: tools/README.txt
##########
@@ -313,7 +313,15 @@ nxstyle.c
standard. This program is completely ignorant of C syntax; it simply
performs crude pattern matching to check the file.
- Usage: nxstyle <path-to-file-to-check>
+ Prints formatted messages that are classified as info, warn, error,
+ fatal. In a parsable format that can be used by editors and IDEs.
+
+ Usage: nxstyle [-m <maxline>] [-v <levele>] <filename>
Review comment:
levele to level
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569514809
I have two functional improvements that I want to make to nxstyle.c, but I will wait for Xiao Xiang to dispose of this PR before I do 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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569512690
@patacongo
First let me say I am Pro NuttX coding style. On Slack, I had stated something to the effect of: ...how incredibly readable code is that conforms to the NuttX coding style and to preserve your legacy, this must be part of the inviolables... I said it better there.
> It is not clear the the disinction between warning, error, and fatal makes any sense for coding standby. isn't is a boolean; The code complies to the standard or it does not. One non-compliance is no better or worse than another.
Good questions!
Here is the short answer to your questions are:
it is fence:
```
FILE *out = stdout;
if (class > INFO)
{
out = stderr;
g_status |= 1;
}
```
>How dif you decide what is an warning, or an error, or fatal. Do you have some criteria for degree of meeting the coding standard?
For the moment
`ERROR("Long line found", lineno, n);` is used in a c file
`WARN("Long line found", lineno, n);` is used in a h file
for maxline
See
```
if (n > maxline)
{
if (g_file_type == C_SOURCE)
{
ERROR("Long line found", lineno, n);
}
else if (g_file_type == C_HEADER)
{
WARN("Long line found", lineno, n);
}
}
```
For a complete understanding.
```
grep -E "(FATAL|FATALFL|WARN|ERROR|INFO)\(\"" nxstyle.c | awk '{$1=$1;print}' | sort
ERROR("Bad alignment", lineno, indent);
ERROR("Bad comment alignment", lineno, indent);
ERROR("Bad comment block alignment", lineno, indent);
ERROR("Bad left brace alignment", lineno, indent);
ERROR("Bad left brace alignment", lineno, indent);
ERROR("Bad right brace alignment", lineno, indent);
ERROR("Blank line contains whitespace", lineno, 1);
ERROR("Blank line follows left brace", lineno, 1);
ERROR("Blank line precedes right brace at line",
ERROR("Block comment terminator must be on a "
ERROR("Carriage returns found. "
ERROR("Carriage returns found. "
ERROR("C comment opening on separate line", lineno, n);
ERROR("Closing C comment not indented", lineno, n);
ERROR("Closing without opening comment", lineno, n);
ERROR("C++ style comment", lineno, n);
ERROR("C++ style comment on at %d:%d\n",
ERROR("Dangling whitespace at the end of line", lineno, n);
ERROR("Expected indentation line", lineno, indent);
ERROR("File begins with a blank line", 1, 1);
ERROR("Functions begin", lineno, n);
ERROR("Garbage follows left bracket", lineno, n);
ERROR("Garbage follows right bracket",
ERROR("Garbage follows right bracket", lineno, n);
ERROR("Garbage on line after C comment", lineno, n);
ERROR("Insufficient indentation line", lineno, indent);
ERROR("Insufficient indentation", lineno, indent);
ERROR("Invalid character after asterisk "
ERROR("Left bracket not on separate line", lineno,
ERROR("Long line found", lineno, n);
ERROR("Missing asterisk in comment block", lineno, indent);
ERROR("Missing asterisk in comment", lineno, indent);
ERROR("Missing blank line after comment", comment_lineno,
ERROR("Missing blank line before comment found", lineno, 1);
ERROR("Missing file header comment block", lineno, 1);
ERROR("Missing space after opening C comment", lineno, n);
ERROR("Missing space before closing C comment", lineno,
ERROR("Missing whitespace after comma", lineno, n);
ERROR("Missing whitespace after keyword", lineno, n);
ERROR("Missing whitespace after keyword", lineno, n);
ERROR("Missing whitespace after keyword", lineno, n);
ERROR("Missing whitespace after semicolon", lineno, n);
ERROR("Mixed case identifier found", lineno, ident_index);
ERROR("Multiple data definitions", lineno, i + 1);
ERROR("Multiple data definitions on line",
ERROR("No indentation line", lineno, indent);
ERROR("No indentation line", lineno, indent);
ERROR("Operator/assignment must be followed with whitespace",
ERROR("Operator/assignment must be preceded "
ERROR("Operator/assignment must be preceded with whitespace",
ERROR("Operator/assignment must be preceded with whitespace",
ERROR("Right brace must be followed by a blank line",
ERROR("Right bracket not on separate line",
ERROR("right left brace alignment", lineno, indent);
ERROR("Small odd alignment", lineno, indent);
ERROR("Space follows left bracket", lineno, n);
ERROR("Space follows left parenthesis", lineno, n);
ERROR("Space precedes right bracket", lineno, n);
ERROR("Space precedes right parenthesis", lineno, n);
ERROR("Space precedes semi-colon",
ERROR("TABs found. First detected", lineno, n);
ERROR("TABs found. First detected", lineno, n);
ERROR("Too many blank lines", lineno, 1);
ERROR("Unmatched right brace", lineno, n);
ERROR("Unmatched right parentheses", lineno, n);
ERROR("Upper case hex constant found", lineno, ident_index);
ERROR("'while' must be on a separate line",
ERROR("Whitespace on blank line", lineno, indent);
ERROR("Whitespace on blank line", lineno, indent);
FATAL("Comment or string found at end of file", lineno, 1);
FATALFL("Failed to open", filename);
FATAL("\"Private/Public Functions\" not found!"
WARN("Long line found", lineno, n);
WARN("No file extension", 0 , 0);
```
[DISCUSSION]
>It is not clear the the disinction between warning, error, and fatal makes any sense for coding standby. isn't is a boolean; The code complies to the standard or it does not. One non-compliance is no better or worse than another.
Agreed - a CS should be more digital than analog. But unfortunately the is not the current reality of the NuttX coding style or the current tools.
The [above comment about](https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422728) `-m` is a good a example of it's analog nature.
Here is another example of it's analog nature.
If we want to grow the Apache NuttX project we need tooling and a CI that works **for us** not **against us**. (When we get to that juncture, I will post something about the poison of a bad CI implementations ) Suffice it to say: having a bad experience on any front, is death to a projects adoption!
Since we want pre-comment hooks and CI to check conformity to NuttX coding style. We have **several issues** to overcome.
A proficient NuttX coding style tool would check for conformance, suggest fixes and optionally apply them.
We do not have a proficient NuttX coding style tool or set of tools yet.
Many of us have had our countless hours of work destroyed by the indent.sh.
### **Remember to NEVER NEVER* run indent.sh on a formatted file**
*Once you've been burned you will learn to commit often and `rebase -i` when it all looks OK.
Historical (see groups and lists) there are a lot posts like this:
>I do see many problems with the code now that would not permit it come into the master branch. It is good you are looking at the coding standing. There are all also tools that can help you in nuttx/tools. You should apply these tools in the following order:
>
>detab.c will replace all of the TABs wtih spaces. No TABs are allowing in .c or .h files.
>rmcr.c will remove whitespace at the end of lines
>convert-comments.c will replace all C++ style comments with C89 style comments as required by >the coding style
>lowhex.c will convert upper case hex constants to lower case hex constants
>indent.sh will get the code close to the NuttX coding system. If used on .h files, however, it tends to >add too much indentation. It also makes a few mistakes
>nxstyle.c will do a final check to make sure that the code is close to the coding style.
If we want to grow the project. This can not continue to be the way we expect "embedded talent" to spend their time. It does not scale: ambiguity and non-deterministic processes wastes a great deal resources, time and effort and will result in many drive-by-pr's or forks.
The sensible conclusion is to remove the human from the loop and automate it with proper tools.
This change is the first step away from [this](https://github.com/apache/incubator-nuttx/blob/master/tools/README.txt#L297-L318) quality of tooling. We have a long way to go.
I have work on a custom version of atsyle, and with @mubes suggestion of [whatstyle](https://github.com/mikr/whatstyle) but I have little time to get this all to converge.
It is my hope that this reporting change in this PR will give us more time to work on proper tooling,
I do feel strongly about it's ultimate integration:
The API should be a simple invocation with NO ROOM for analog setting or not checking what has been changed!
This got batted down previously:
```
make check_format
make format
```
but for all the right reasons `make` is the place it goes. Remember @patacongo 's argument that user is the customer. He is right and for a for a community to grow the barrier to entry need to be low.
Having a bad first experience is death to a projects adoption.
REMEMBER: The N00B can use the make command and the PRO can use the script - we are not forcing our 'will' on anyone we are enabling the community to grow and succeed.
There is a great line in [Patch Adams](https://www.youtube.com/watch?v=9pFvM21Ng14) @0:398 ish
seconds that comes to mind often in theses discussion..."If You Were Right, I Would Agree With You"
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362011922
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
Review comment:
Done!
----------------------------------------------------------------
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] xiaoxiang781216 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569756871
@patacongo, @davids5 fix resolve my most comment except snprintf, but that is a very minor issue. The patch is good for me, you can squash into master or nxstyle.
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569514809
@xiaoxiang781216 I have two functional improvements that I want to make to nxstyle.c, but I will wait for Xiao Xiang to dispose of this PR before I do 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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362010914
##########
File path: tools/nxstyle.c
##########
@@ -134,70 +211,80 @@ int main(int argc, char **argv, char **envp)
int rbrace_lineno; /* Last line containing a right brace */
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
- int maxline; /* Lines longer that this generate warnings */
int n;
int i;
+ int c;
- maxline = 78;
- filename = argv[1];
-
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ while ((c = getopt(argc, argv, ":hv:gm:")) != -1)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
- }
- else if (argc == 2)
- {
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ g_maxline = atoi(optarg);
+ if (g_maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 'v':
+ g_verbose = atoi(optarg);
+ if (g_verbose < 0 || g_verbose > 2)
+ {
+ show_usage(argv[0], 1, "Bad value for <level>.");
+ }
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
- instream = fopen(filename, "r");
+ g_file_name = argv[optind];
+ instream = fopen(g_file_name, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ FATALFL("Failed to open", g_file_name);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ hdrfile = false;
+ g_file_type = UNKNOWN;
Review comment:
don't need?
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422025
I have already review this in its previous incarnation. So I give my approval nw. I don't know if we can approvals from the other people on the list.
There is no workflow governing tools in nuttx/tools or apps/tools. C tools should follow C coding standard and the committer should satisfy themselves that the changes are functional. These change does not seem to affect the coding standard verification logic, only the form of error reporting so it is low risk.
When there are changes to nxstyle that do affect the detection of problems, there is some very careful testing should be done to (1) assure that the change detects the problem and (2) does not introduce false alarms. I can share how I do that when the time is right.
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361811349
##########
File path: tools/README.txt
##########
@@ -313,7 +313,15 @@ nxstyle.c
standard. This program is completely ignorant of C syntax; it simply
performs crude pattern matching to check the file.
- Usage: nxstyle <path-to-file-to-check>
+ Prints formatted messages that are classified as info, warn, error,
+ fatal in parsable format that can be used by editors and IDEs.
Review comment:
```suggestion
fatal. In a parsable format that can be used by editors and IDEs.
```
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007456
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
Review comment:
Done! ea8b41d8db
----------------------------------------------------------------
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] xiaoxiang781216 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569381487
@liuguo09 please take a look.
----------------------------------------------------------------
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 a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362005445
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
Review comment:
You have good eyes. Yes, those externs must go.
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361978161
##########
File path: tools/README.txt
##########
@@ -313,7 +313,15 @@ nxstyle.c
standard. This program is completely ignorant of C syntax; it simply
performs crude pattern matching to check the file.
- Usage: nxstyle <path-to-file-to-check>
+ Prints formatted messages that are classified as info, warn, error,
+ fatal. In a parsable format that can be used by editors and IDEs.
+
+ Usage: nxstyle [-m <maxline>] [-s] [-g] <path-to-file-to-check>
+ Where -m <maxline> is the maximum line length allowed
+ Where -s silent prints not output, returns 0 for OK and 1
+ for any errors.
+ Where -g go-no-go output only prints PASS/FAIL gor a
Review comment:
```suggestion
Where -g go-no-go output only prints PASS/FAIL for a
```
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362017755
##########
File path: tools/nxstyle.c
##########
@@ -386,24 +467,26 @@ int main(int argc, char **argv, char **envp)
{
if (!bcrs)
{
- fprintf(stderr, "Carriage returns found. "
- "First detected at line %d:%d\n", lineno, n);
+ ERROR("Carriage returns found. "
+ "First detected", lineno, n);
bcrs = true;
}
}
break;
default:
{
- fprintf(stderr,
- "Unexpected white space character %02x found at line %d:%d\n",
- line[n], lineno, n);
+ snprintf(buffer, sizeof(buffer),
Review comment:
why we can't ERROR directly?
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569758903
@xiaoxiang781216 Please, make whatever additional changes to you want to make against branch nxstyle and merge that branch to master.
----------------------------------------------------------------
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] davids5 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569500865
How to use nxstyle with **style**
Eclipse with CDT (https://gnu-mcu-eclipse.github.io/) Empowered development with SVD enabled debugging.
Setup Tab "Builder Setting"
Setup Tab "Behavior"
tool in action
select a file (open it)
Select nxstyle as the tool
The Clck the Hammer!
Get a list of CS violations
Click and fix them! (hit work bottom up)
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362035777
##########
File path: tools/nxstyle.c
##########
@@ -669,25 +757,25 @@ int main(int argc, char **argv, char **envp)
{
if (!btabs)
{
- fprintf(stderr, "TABs found. First detected at line %d:%d\n",
- lineno, n);
+ ERROR("TABs found. First detected", lineno, n);
btabs = true;
}
}
else if (line[n] == '\r')
{
if (!bcrs)
{
- fprintf(stderr, "Carriage returns found. "
- "First detected at line %d:%d\n", lineno, n);
+ ERROR("Carriage returns found. "
+ "First detected", lineno, n);
bcrs = true;
}
}
else if (line[n] != ' ')
{
- fprintf(stderr,
- "Unexpected white space character %02x found at line %d:%d\n",
- line[n], lineno, n);
+ snprintf(buffer, sizeof(buffer),
Review comment:
[Ditto](https://github.com/xiaoxiang781216)
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361990957
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
+ g_file_name = filename;
instream = fopen(filename, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ g_gonogo = false;
Review comment:
why special handle this case? if user don't want to see anything, let's follow them.
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362020748
##########
File path: tools/nxstyle.c
##########
@@ -669,25 +757,25 @@ int main(int argc, char **argv, char **envp)
{
if (!btabs)
{
- fprintf(stderr, "TABs found. First detected at line %d:%d\n",
- lineno, n);
+ ERROR("TABs found. First detected", lineno, n);
btabs = true;
}
}
else if (line[n] == '\r')
{
if (!bcrs)
{
- fprintf(stderr, "Carriage returns found. "
- "First detected at line %d:%d\n", lineno, n);
+ ERROR("Carriage returns found. "
+ "First detected", lineno, n);
bcrs = true;
}
}
else if (line[n] != ' ')
{
- fprintf(stderr,
- "Unexpected white space character %02x found at line %d:%d\n",
- line[n], lineno, n);
+ snprintf(buffer, sizeof(buffer),
Review comment:
Why format to buffer first? can we call ERROR directly?
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361989974
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
Review comment:
Should we set maxline to 86 by default?
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569743134
@xiaoxiang781216 Perhaps you should merge this onto a feature branch, perhaps called 'nxstyle'. There we can work together to continue development. It is currently based on the master branch. You can create an nxstyle branch and change the PR base branch per https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-base-branch-of-a-pull-request
We can then cooperate with the remaining changes requested, Haitao Liu's changes, and the things that I want to do.
NOTE: Any further changes to the feature branch must also be made via PR, not by direct merges to the branch.
PS: I can help with any of this. But I think you are in charge of the disposition of the PR.
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569514635
This one is a little misleading:
FATAL("\"Private/Public Functions\" not found!"
There is a "switch" (a boolean) that changes the behavior when the tool reaches the code in the file. It detects this point whn the Private or Public Functions block comment is encountered. It is misleading on C files that contain only global data definitions. For thoses files, there will naturally be no such block comments and and perfectly fine code would generate this fatal error.
----------------------------------------------------------------
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 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569422728
The use of '-m 99' in the testing is a little odd. I usuaully use -m 86. This probably raises some tool use questions. The coding standard says:
Line Width. Text should not extend past column 78 in the typical C source or header file. Sometimes the nature of the content of a file may require that the lines exceed this limit. This often occurs in header files with naturally long definitions. If the line width must extend 78 lines, then some wider line width may be used in the file provided that it is used consistently.
This should match the width of the block comments:
Block Comments. Each grouping in the file is separated with a block comment. The block comment consists of:
- A line that consists of the opening C comment (/*) followed by a series of asterisks extending to the length of the line (usually to column 78).
- The name of the grouping, starting at column 4. An asterisk preceives the name of the grouping in column 1.
- A line that consists of the closing C comment (*/) at the end of the line (usually column 78) preceded by a series of asterisks extending to column 1.
So picking any fixed value will cause long lines to be missed OR many invalid "long line" error reports. Ideally, we should parse the file first, look at the block comments, make sure that they are the same width, and set the maximum line with automatically.
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569527398
@patacongo. I'm glad you have some improvements.
Your options are to:
1 wait
OR
2 do a PR against this Branch in this repo
OR
3 I give you write access to this repo and you can pull this branch and add
your commits on it.
David
On Sun, Dec 29, 2019, 7:18 AM patacongo <no...@github.com> wrote:
> I have two functional improvements that I want to make to nxstyle.c, but I
> will wait for Xiao Xiang to dispose of this PR before I do those.
>
> —
> You are receiving this because you authored the thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/apache/incubator-nuttx/pull/12?email_source=notifications&email_token=AAO3BXNBIRYH2UJDDYJFCW3Q3C5TNA5CNFSM4KACHTU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEHZBWOI#issuecomment-569514809>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/AAO3BXML4P2HHF3T4SJK74LQ3C5TNANCNFSM4KACHTUQ>
> .
>
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362007238
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
Review comment:
Done!
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362014598
##########
File path: tools/nxstyle.c
##########
@@ -134,70 +211,80 @@ int main(int argc, char **argv, char **envp)
int rbrace_lineno; /* Last line containing a right brace */
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
- int maxline; /* Lines longer that this generate warnings */
int n;
int i;
+ int c;
- maxline = 78;
- filename = argv[1];
-
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ while ((c = getopt(argc, argv, ":hv:gm:")) != -1)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
- }
- else if (argc == 2)
- {
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ g_maxline = atoi(optarg);
+ if (g_maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 'v':
+ g_verbose = atoi(optarg);
+ if (g_verbose < 0 || g_verbose > 2)
+ {
+ show_usage(argv[0], 1, "Bad value for <level>.");
+ }
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
- instream = fopen(filename, "r");
+ g_file_name = argv[optind];
+ instream = fopen(g_file_name, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ FATALFL("Failed to open", g_file_name);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ hdrfile = false;
+ g_file_type = UNKNOWN;
Review comment:
Done!
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361979048
##########
File path: tools/nxstyle.c
##########
@@ -1896,24 +1962,32 @@ int main(int argc, char **argv, char **envp)
else if (indent == 1 || indent == 3)
{
- fprintf(stderr, "Small odd alignment at line %d:%d\n",
- lineno, indent);
+ ERROR("Small odd alignment", lineno, indent);
}
}
}
}
if (!bfunctions && !hdrfile)
{
- fprintf(stderr, "ERROR: \"Private/Public Functions\" not found!\n");
- fprintf(stderr, " File could not be checked.\n");
+ FATAL("\"Private/Public Functions\" not found!"
Review comment:
```suggestion
ERROR("\"Private/Public Functions\" not found!"
```
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361986470
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
Review comment:
opt* Already declared in unistd.h, why need again?
----------------------------------------------------------------
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] davids5 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569500865
# How to use nxstyle with **style**
Eclipse with CDT (https://gnu-mcu-eclipse.github.io/) Empowered development with SVD enabled debugging.
## Setup Tab "Builder Setting"
## Setup Tab "Behavior"
## Tool in action
- select a file (open it)
- Select nxstyle as the tool
- The Clck the Hammer!
## Get a list of CS violations
## Click and fix them! (hint work bottom up)
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362035720
##########
File path: tools/nxstyle.c
##########
@@ -386,24 +467,26 @@ int main(int argc, char **argv, char **envp)
{
if (!bcrs)
{
- fprintf(stderr, "Carriage returns found. "
- "First detected at line %d:%d\n", lineno, n);
+ ERROR("Carriage returns found. "
+ "First detected", lineno, n);
bcrs = true;
}
}
break;
default:
{
- fprintf(stderr,
- "Unexpected white space character %02x found at line %d:%d\n",
- line[n], lineno, n);
+ snprintf(buffer, sizeof(buffer),
Review comment:
@xiaoxiang781216 -
To make this conform to a very simple API and not lose any of Greg's work in formatting.
I will not be investing any more time on this.
Please feel free to have your team make any further changes like adding var args it xiaimi needs them.
----------------------------------------------------------------
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] davids5 edited a comment on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 edited a comment on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569500865
How to use nxstyle with **style**
Eclipse with CDT (https://gnu-mcu-eclipse.github.io/) Empowered development with SVD enabled debugging.
## Setup Tab "Builder Setting"
Setup Tab "Behavior"
## Tool in action
- select a file (open it)
- Select nxstyle as the tool
- The Clck the Hammer!
## Get a list of CS violations
## Click and fix them! (hit work bottom up)
----------------------------------------------------------------
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] davids5 commented on issue #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569500865
How to use nxstyle with **style**
Eclipse with CDT (https://gnu-mcu-eclipse.github.io/) Empowered development with SVD enabled debugging debugging.
Setup Tab "Builder Setting"
Setup Tab "Behavior"
tool in action
select a file (open it)
Select nxstyle as the tool
The Clck the Hammer!
Get a list of CS violations
Click and fix them! (hit work bottom up)
----------------------------------------------------------------
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] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361987147
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
Review comment:
remove filename, always use g_file_name?
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362011999
##########
File path: tools/nxstyle.c
##########
@@ -135,67 +211,98 @@ int main(int argc, char **argv, char **envp)
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
int maxline; /* Lines longer that this generate warnings */
+ bool silent; /* Used with go not go test option */
int n;
int i;
+ int c;
+ extern char *optarg;
+ extern int optopt;
+ extern int optind;
- maxline = 78;
- filename = argv[1];
+ g_gonogo = false;
+ maxline = 78;
+ filename = argv[1];
+ silent = false;
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ if (argc < 2)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
+ show_usage(argv[0], 1, "No file name given.");
}
- else if (argc == 2)
+
+ while ((c = getopt(argc, argv, ":hsgm:")) != -1)
{
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ maxline = atoi(optarg);
+ if (maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 's':
+ silent = true;
+ break;
+
+ case 'g':
+ g_gonogo = true;
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
+ filename = argv[optind];
Review comment:
done!
----------------------------------------------------------------
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 #12: nxstyle
improvements with No tooling
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#issuecomment-569444969
Just noticed... tools/README.txt also needs to be updated. I think just the old Usage comments need to be replaced with the new Usage 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
With regards,
Apache Git Services
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on a change in pull
request #12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362020988
##########
File path: tools/nxstyle.c
##########
@@ -134,70 +213,78 @@ int main(int argc, char **argv, char **envp)
int rbrace_lineno; /* Last line containing a right brace */
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
- int maxline; /* Lines longer that this generate warnings */
int n;
int i;
+ int c;
- maxline = 78;
- filename = argv[1];
-
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ while ((c = getopt(argc, argv, ":hv:gm:")) != -1)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
- }
- else if (argc == 2)
- {
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ g_maxline = atoi(optarg);
+ if (g_maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 'v':
+ g_verbose = atoi(optarg);
+ if (g_verbose < 0 || g_verbose > 2)
+ {
+ show_usage(argv[0], 1, "Bad value for <level>.");
+ }
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
- instream = fopen(filename, "r");
+ g_file_name = argv[optind];
+ instream = fopen(g_file_name, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ FATALFL("Failed to open", g_file_name);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ g_file_type = UNKNOWN;
Review comment:
remove the assignment, not needed at all.
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r362035976
##########
File path: tools/nxstyle.c
##########
@@ -134,70 +213,78 @@ int main(int argc, char **argv, char **envp)
int rbrace_lineno; /* Last line containing a right brace */
int externc_lineno; /* Last line where 'extern "C"' declared */
int linelen; /* Length of the line */
- int maxline; /* Lines longer that this generate warnings */
int n;
int i;
+ int c;
- maxline = 78;
- filename = argv[1];
-
- /* Usage: nxstyle [-m <maxline>] <filename>
- * nxstyle -h
- */
-
- if (argc == 4)
+ while ((c = getopt(argc, argv, ":hv:gm:")) != -1)
{
- if (strcmp(argv[1], "-m") != 0)
- {
- fprintf(stderr, "Unrecognized argument\n");
- show_usage(argv[0], 1);
- }
-
- maxline = atoi(argv[2]);
- if (maxline < 1)
- {
- fprintf(stderr, "Bad value for <maxline>\n");
- show_usage(argv[0], 1);
- }
-
- filename = argv[3];
- }
- else if (argc == 2)
- {
- if (strcmp(argv[1], "-h") == 0)
- {
- show_usage(argv[0], 0);
- }
- }
- else
+ switch (c)
+ {
+ case 'm':
+ g_maxline = atoi(optarg);
+ if (g_maxline < 1)
+ {
+ show_usage(argv[0], 1, "Bad value for <maxline>.");
+ }
+ break;
+
+ case 'v':
+ g_verbose = atoi(optarg);
+ if (g_verbose < 0 || g_verbose > 2)
+ {
+ show_usage(argv[0], 1, "Bad value for <level>.");
+ }
+ break;
+
+ case 'h':
+ show_usage(argv[0], 0, NULL);
+ break;
+
+ case ':':
+ show_usage(argv[0], 1, "Missing argument.");
+ break;
+
+ case '?':
+ show_usage(argv[0], 1, "Unrecognized option.");
+ break;
+
+ default:
+ show_usage(argv[0], 0, NULL);
+ break;
+ }
+ }
+
+ if (optind < argc - 1 || argv[optind] == NULL)
{
- fprintf(stderr, "Invalid number of arguments\n");
- show_usage(argv[0], 1);
+ show_usage(argv[0], 1, "No file name given.");
}
- instream = fopen(filename, "r");
+ g_file_name = argv[optind];
+ instream = fopen(g_file_name, "r");
+
if (!instream)
{
- fprintf(stderr, "Failed to open %s\n", argv[1]);
+ FATALFL("Failed to open", g_file_name);
return 1;
}
/* Are we parsing a header file? */
- hdrfile = false;
- ext = strrchr(filename, '.');
+ g_file_type = UNKNOWN;
Review comment:
```suggestion
```
----------------------------------------------------------------
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] davids5 commented on a change in pull request
#12: nxstyle improvements with No tooling
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #12: nxstyle improvements with No tooling
URL: https://github.com/apache/incubator-nuttx/pull/12#discussion_r361811329
##########
File path: tools/README.txt
##########
@@ -313,7 +313,15 @@ nxstyle.c
standard. This program is completely ignorant of C syntax; it simply
performs crude pattern matching to check the file.
- Usage: nxstyle <path-to-file-to-check>
+ Prints formatted messages that are classified as info, warn, error,
+ fatal in parsable that can be used by editors and IDEs.
Review comment:
```suggestion
fatal in parsable format that can be used by editors and IDEs.
```
----------------------------------------------------------------
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