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++ Fix for #47 #49

Merged
merged 9 commits into from
Apr 1, 2019
Merged

C++ Fix for #47 #49

merged 9 commits into from
Apr 1, 2019

Conversation

matter123
Copy link
Collaborator

This provides a C++ fix for #47.

Additionally, this improves highlighting of inherited base classes. Previously only 1 base class per access specifier was allowed. This PR extends that to any number of base classes.

1.6.6 This PR
Screen Shot 2019-03-31 at 7 50 49 PM Screen Shot 2019-03-31 at 7 52 21 PM

This does not fix the same issue in the C grammar.

@matter123 matter123 changed the title C++ Fix #47 C++ Fix for #47 Apr 1, 2019
Copy link
Owner

@jeff-hykin jeff-hykin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added tags to the colon and the commas. Other than that my changes were basically just moving code around. Great job 👍

@jeff-hykin jeff-hykin merged commit 07d9f23 into master Apr 1, 2019
@matter123
Copy link
Collaborator Author

matter123 commented Apr 1, 2019

You shouldn't need the complicated Regex on https://github.com/jeff-hykin/cpp-textmate-grammar/blob/b1b034011874d96e10ecc9a42a87771fbe800748/generate.rb#L891 patterns are matched in order so : is excluded from the includes, , and access specifiers have already been matched and are excluded from the regex on Line 891. All that is left is spaces and variable_names. I think the current regex on Line 891 tags the spaces as part of the inherited class name.

@jeff-hykin
Copy link
Owner

jeff-hykin commented Apr 1, 2019

Yeah for your original one you're right it's not needed. I abstracted out the inhertance_context though so it could be used in two different places, and the second place is when the complex regex is needed.

Right now:

With the complicated regex:
Screen Shot 2019-04-01 at 2 02 45 PM

Without the complicated regex:
Screen Shot 2019-04-01 at 2 03 31 PM

@jeff-hykin
Copy link
Owner

jeff-hykin commented Apr 1, 2019

The reason I did the abstraction is because your pattern matching was better, and it could be used in 2nd place.

I appreciate you looking over the changes though 👍

@matter123
Copy link
Collaborator Author

Ah, okay. You might still want to fix the regex tagging the space before the inherited class, however.

@jeff-hykin
Copy link
Owner

jeff-hykin commented Apr 1, 2019

Good call, I missed that. Next time I'll post my changes for review Haha.

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.

None yet

2 participants