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

C/C++ preprocessor if in enum #299

Closed
alexr00 opened this issue Jul 10, 2019 · 4 comments
Closed

C/C++ preprocessor if in enum #299

alexr00 opened this issue Jul 10, 2019 · 4 comments
Labels
🐛 Bug Something isn't working Hard

Comments

@alexr00
Copy link

alexr00 commented Jul 10, 2019

From @pfaltynek-als in microsoft/vscode#77046

The scopes of the #if and #endif in the enum are not correct:

enum FOO {
    ONE,
    TWO,
#if COND
    TWO_AND_HALF,
#endif
    THREE
}

#if COND
#define TWO_AND_HALF 1
#endif
@matter123 matter123 added 🐛 Bug Something isn't working Hard labels Jul 10, 2019
@matter123
Copy link
Collaborator

matter123 commented Jul 10, 2019

The simple fix of adding :ever_present_context to the body_includes of enums fixes the issue of highlighting of preprocessor statements. However, TWO_AND_HALF is no longer marked as variable.other.enummember.cpp.

@jeff-hykin is there a reason that preprocessor statements try to find there #else or #endif? What features would be lost if the preprocessor highlighting just treated each line independently? Alternatively, the preprocessor needs to be rewritten, preferably as a shared_pattern, to accept a context for the inner portion.

Edit: one feature that would be lost is that the grammar treats

#if 0
This is a comment
#endif

as a multi line comment

@jeff-hykin
Copy link
Owner

Legacy support is the only reason the preprocessor conditionals are treated as ranges.

we can add a special pattern to find the #if 0 range and treat it as a comment.

I think the shared pattern idea would work well. Everytime the :ever_present_context is included it could be replaced with a function call that generates a new preprocessor if range that includes the parent context. We'd basically have to create an extra pattern range for every existing range but I don't think that would be an issue.

the only problem I'd be concerned about is if a start pattern is triggered inside of the if statement

#if blah
func(
#else 
func2(
#endif

The if statement should probably use a while instead of an end pattern so that it can force close any scopes that were opened.

However, with an if-then-else statement at least one of the cases should be allowed to overflow since one of them is required. Given that textmate has no idea if it is and if or if-then-else statement, the first one should probably always be closed, and then the else statement should be left open.

This doesn't seem too difficult and I think it would result in near perfect highlighting. I might try implementing it tonight.

@matter123
Copy link
Collaborator

Sounds good 👍

@jeff-hykin
Copy link
Owner

jeff-hykin commented Jul 12, 2019

The initial fix was posted with v1.12.19, I wasn't able to get around to the near-perfect method. I think this issue can be closed since #31 was made to track the near-perfect solution anyways.

I'm somewhat concerned that the near-perfect solution will indirectly break a lot of code because of the external/upstream problems with while. If people put their entire program inside of an if statement it could wreck their whole syntax.

I'm leaving tomorrow and won't have access to my computer until July 23rd. So I didn't want to push anything potentially breaking right before leaving. I will still be able to approve merges on my phone, but I won't have a good way to publish the syntax.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working Hard
Projects
None yet
Development

No branches or pull requests

3 participants