Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

comment: C, Cpp and Css styles reorganization (big) #941

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

Thannoy
Copy link

@Thannoy Thannoy commented Mar 30, 2024

C and C++ features different comment styles:

  • only multi-line /* */ comment is valid in traditional C89
  • C++ also supports // single line comments from its beginning
  • C started to support C++-style // comments in C99

For this reason, multi-line /* */ is often named C-style comment and single-line // is often named C++-style comment.

In current implementation, there were only a "CCommentStyle" with /* */ and // support, used for both C and C++ files (also used by quite many other file types).

Therefore:

  • Rename CCommentStyle as CppCommentStyle
  • Create a new CCommentStyle with only /* */ support
  • Remove redundant CssCommentStyle (same as new CCommentStyle)
  • Associate *.c and *.h with CCommentStyle

Caution:

  • this (slightly) changes *.c and *.h comments (by removing their multi-line support)
    • despite *.h files being often used also for C++, which is a minor issue in my opinion since C comments is a subset of C++ comment
  • this changes the list of known style shorthands:
    • "c" style shorthand meaning changes
    • "css" style shorthand is no more known (want we keep a redundant CssCommentStyle to avoid this?)

This PR is a little bit big but in my opinion results in a clearer handling of C/C++ (and Css).

@Thannoy
Copy link
Author

Thannoy commented Mar 30, 2024

Another caution about this PR: since it touches quite many places, conflicts with other PRs are likely.

@Thannoy
Copy link
Author

Thannoy commented Jun 30, 2024

  • Resync'ed with main 1a131cc
  • added missing cppsingle shorthand update

@Thannoy Thannoy changed the title comment: C/Cpp/Css comment styles reorganisation comment: C, Cpp and Css styles reorganization (big) Jun 30, 2024
@Thannoy
Copy link
Author

Thannoy commented Jun 30, 2024

For information purpose, resyncing this PR with master updates is not that trivial. Here is my workflow:

  • git fetch
  • git merge origin/main
  • re-do comment.py d073a5 key modifications:
    • git checkout origin/main -- src/reuse/comment.py
    • rename all occurrences of CCommentStyle to CppCommentStyle
    • rename all occurrences of CssCommentStyle to CCommentStyle
    • fix impacted *CommentStyle themselves and their shorthands (C/Cpp/CppSingle/Css)
    • change .c and .h comment style to CCommentStyle
  • tests

@carmenbianca carmenbianca force-pushed the tmp/c-cpp-styles branch 2 times, most recently from fd8fe1b to d77b70a Compare July 1, 2024 09:57
Anthony Loiseau and others added 2 commits July 1, 2024 13:33
C and C++ features different comment styles:
- only multiline /* */ comment is valid in tradional C89
- C++ also supports // single line comments
- C started to support // comments in C99

For this reason, /* */ is often named C-style comment
and // is often named C++-style comment.

Therefore:
- Rename CCommentStyle as CppCommentStyle
- Create a new CCommentStyle with only /* */ support
- Remove redundant CssCommentStyle (same as new CCommentStyle)

Caution, this changes the list of known style shorthands:
- "c" style shorthand meaning changes
- "css" style shorthand is no more known

Signed-off-by: Anthony Loiseau <anthony@loiseau.fr>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
Signed-off-by: Carmen Bianca BAKKER <carmenbianca@fsfe.org>
@carmenbianca
Copy link
Member

Hi @Thannoy and thanks! I rebased on top of master again, and did some more adjusting. To maintain support for the old shorthands, I manually added them to the dictionary and added tests for this.

This PR is technically a breaking change still, but I can live with that.

Thanks for the work and the history context.

@carmenbianca carmenbianca merged commit 21e7005 into fsfe:main Jul 1, 2024
15 checks passed
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.

2 participants