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

Add disambiguation for C++ header files #1269

Merged

Conversation

pyrmont
Copy link
Contributor

@pyrmont pyrmont commented Jul 22, 2019

C++ header files can use the .h extension. By default, Rouge highlights these files with the C lexer. This can lead to incorrect highlighting because certain keywords in C++ are not reserved in C. A similar situation can occur with Objective-C files but Rouge provides a disambiguation for Objective-C files to avoid this problem. This commit adds a similar disambiguation for C++.

This fixes #932.

C++ header files can use the `.h` extension. By default, Rouge
highlights these files with the C lexer. This can lead to incorrect
highlighting because certain keywords in C++ are not reserved in C. A
similar situation can occur with Objective-C files but Rouge provides a
disambiguation for Objective-C files to avoid this problem. This commit
adds a similar disambiguation for C++.
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Jul 22, 2019
@eliaskosunen
Copy link

eliaskosunen commented Jul 22, 2019

The list of keywords could be improved. bool is technically a reserved word in C (it's a macro #defined to _Bool in the header <stdbool.h>), so that probably shouldn't be on the list. Other keywords that can appear at the beginning of a line present in C++, but are absent in C include, off the top of my head:

  • constexpr
  • try, catch and throw
  • protected
  • noexcept
  • static_cast, dynamic_cast and reinterpret_cast
  • virtual
  • this
  • typeid
  • thread_local
  • decltype
  • static_assert
  • alignas and alignof
  • As of C++20, concept, requires, co_await, co_async, co_yield and consteval

and probably more

Whether these keywords should be used for detection, I don't know.

@pyrmont
Copy link
Contributor Author

pyrmont commented Jul 22, 2019

@eliaskosunen Thanks for the quick reply! Would some of those really begin a line in a header file (eg. this, try)? I think the key thing is to identify keywords that may be present in a header file on their own. Based on what I've seen, something like namespace meets this criterion whereas typeid doesn't. Of that list, I'm inclined to want to add protected (since I already added public and private) and virtual. But perhaps that's being too conservative?

@eliaskosunen
Copy link

Headers can contain inline function definitions, and with funky formatting, all of these could potentially appear at the beginning of a line:

struct S {
  void foo() {
    try { // `try` as the first word of a line
      bar(
        this); // `this` as the first word of a line
    } catch(...) {
      // ...
    }
  }
};

Should you want to stay conservative, in the least I think you should have, in addition of what you listed, constexpr (appears frequently as the first word of a function signature in post-C++14-world), try, catch and throw.

@pyrmont
Copy link
Contributor Author

pyrmont commented Jul 22, 2019

@eliaskosunen Ah, forgot about inline function definitions. Hmmm... I think what I'd like to do at this point is release this fix and see how it goes in everyday use. What do you think?

@eliaskosunen
Copy link

This would certainly be better than what we have now.

@pyrmont pyrmont merged commit 9247133 into rouge-ruby:master Jul 24, 2019
@pyrmont
Copy link
Contributor Author

pyrmont commented Jul 24, 2019

@eliaskosunen Thanks for your help on this. We're on a two-week release cadence at the moment as we work through the backlog so this will go out as part of v3.8.0 on Tuesday 6 August. I'm not affiliated with GitLab at all so am not sure when they'll switch over to using it so you might want to check with them.

@pyrmont pyrmont removed the needs-review The PR needs to be reviewed label Jul 24, 2019
@jneen
Copy link
Member

jneen commented Jul 24, 2019

@eliaskosunen Back when I worked for Gitlab, I implemented this, which may be of interest to you: https://docs.gitlab.com/ce/user/project/highlighting.html

You can specify the language for any glob of files.

@pyrmont pyrmont deleted the bugfix.cpp-header-file-disambiguation branch January 8, 2020 20:08
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.

C++ header files are not highlighted correctly
3 participants