Fix #671: Missing newline at end of file should produce a warning by glankk · Pull Request #672 · cppcheck-opensource/simplecpp · GitHub
Skip to content

Fix #671: Missing newline at end of file should produce a warning - #672

Merged
glankk merged 4 commits into
cppcheck-opensource:masterfrom
glankk:671
Jul 20, 2026
Merged

glankk merged 4 commits into
cppcheck-opensource:masterfrom
glankk:671

Conversation

@glankk

@glankk glankk commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@firewave firewave linked an issue Jul 9, 2026 that may be closed by this pull request
@firewave

firewave commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

@danmar

danmar commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

All these stray -1 and \n are really awful and make this feel very wrong.

I am not sure why the -1 are needed. Not saying it's wrong, I just don't understand it.

The \n makes the testcode technically valid. Are you suggestion to remove those and ensure that warnings are written properly?

@firewave

firewave commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

I am not sure why the -1 are needed. Not saying it's wrong, I just don't understand it.

Me neither. Passing less means that the newline would be missing and the other changes add it. That is kind of a flip-flop.

The \n makes the testcode technically valid. Are you suggestion to remove those and ensure that warnings are written properly?

If fiddling with the trailing newline is necessary that seems like a possible source of issues for users of the library.

@glankk

glankk commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

I am not sure why the -1 are needed. Not saying it's wrong, I just don't understand it.

The char arrays that are initialized with a string constant include a null terminator, which makes these test cases fail because they end with '\0' instead of '\n'.

All these stray -1 and \n are really awful and make this feel very wrong.

I agree that it looks really ugly but it's not wrong.

@glankk

glankk commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

I also agree that it's a bit awkward to implement this in simplecpp, since it doesn't really distinguish between C and C++.

@firewave

Copy link
Copy Markdown
Collaborator

I am not sure why the -1 are needed. Not saying it's wrong, I just don't understand it.

The char arrays that are initialized with a string constant include a null terminator, which makes these test cases fail because they end with '\0' instead of '\n'.

All these stray -1 and \n are really awful and make this feel very wrong.

I agree that it looks really ugly but it's not wrong.

Maybe we should something more explicit to highlight what we are doing and it doesn't look like we randomly added those. We should probably land #666 first which allows passing the actual sizes instead of just relying onto strlen().

@danmar

danmar commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

If fiddling with the trailing newline is necessary that seems like a possible source of issues for users of the library.

Of course we should write test code properly. If the newline is not included in the test code then it is not included.
Let's write test code exactly as we want it to be.

All these stray -1 and \n are really awful and make this feel very wrong.
I agree that it looks really ugly but it's not wrong.

It's not "ugly" to write test code properly.

I understand now why you write sizeof(code)-1 and that is the proper way to calculate the size.

I.e. if we have const char code[] = "abcde"; then sizeof(code) will return 6. If we pass 6 as size argument to the preprocess function then we tell the preprocess function that it should read the null character after the 'e' when preprocessing. If we don't intended that then we should not pass 6.

@danmar

danmar commented Jul 12, 2026

Copy link
Copy Markdown
Collaborator

Comment thread main.cpp
@glankk
glankk merged commit f6947eb into cppcheck-opensource:master Jul 20, 2026
26 checks passed
@glankk
glankk deleted the 671 branch July 20, 2026 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing newline at end of file should produce a warning

3 participants