Skip to content

C++: Add enum class to class names#2342

Merged
RunDevelopment merged 5 commits into
PrismJS:masterfrom
jsmolka:master
Apr 29, 2020
Merged

C++: Add enum class to class names#2342
RunDevelopment merged 5 commits into
PrismJS:masterfrom
jsmolka:master

Conversation

@jsmolka

@jsmolka jsmolka commented Apr 29, 2020

Copy link
Copy Markdown
Contributor

Prism falsy indentifies the class in enum class as a class name and therefore applies the wrong color.

image

This commit fixes it. The order, before enum itself, is important.

image

RunDevelopment
RunDevelopment reviewed Apr 29, 2020
View reviewed changes
Comment thread components/prism-cpp.js Outdated
@jsmolka @RunDevelopment
C++: disallow keyword class names
ae2ecca 
Co-Authored-By: Michael Schmidt <[email protected]>
@RunDevelopment

RunDevelopment commented Apr 29, 2020

Copy link
Copy Markdown
Member

Thank you for this fix @jsmolka!

To make the build pass, you also have to commit the minified files. Install Prism's dependencies using npm ci, run npx gulp, and commit all changes. The build should now pass.

Please also add the previous error to this test.

@jsmolka

jsmolka commented Apr 29, 2020 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

@RunDevelopment I have already tried altering the test file in the following way

class Foo
class Foo_bar
struct foo
enum bar
enum class FooBar

----------------------------------------------------

[
	["keyword", "class"], ["class-name", "Foo"],
	["keyword", "class"], ["class-name", "Foo_bar"],
	["keyword", "struct"], ["class-name", "foo"],
	["keyword", "enum"], ["class-name", "bar"],
	["keyword", "enum class"], ["class-name", "FooBar"]
]

----------------------------------------------------

Checks for class names.

After running gulp and then npm run test I got

: expected [ Array(11) ] to deeply equal [ Array(10) ]
      + expected - actual

           "bar"
         ]
         [
           "keyword"
      -    "enum"
      +    "enum class"
         ]
         [
      -    "keyword"
      -    "class"
      -  ]
      -  [
           "class-name"
           "FooBar"
         ]
       ]

@RunDevelopment

RunDevelopment commented Apr 29, 2020

Copy link
Copy Markdown
Member

Yes, the test suite because the expected JSON wasn't updated. Use npm run test:languages -- --pretty to get correct formatted JSON.

@jsmolka

jsmolka commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

The test still fails :/

@RunDevelopment

RunDevelopment commented Apr 29, 2020

Copy link
Copy Markdown
Member

The correct JSON is probably this:

[
	["keyword", "class"], ["class-name", "Foo"],
	["keyword", "class"], ["class-name", "Foo_bar"],
	["keyword", "struct"], ["class-name", "foo"],
	["keyword", "enum"], ["class-name", "bar"],
	["keyword", "enum"], ["keyword", "class"], ["class-name", "FooBar"]
]

@jsmolka

jsmolka commented Apr 29, 2020

Copy link
Copy Markdown
Contributor Author

That fixed it, thanks!

@RunDevelopment
RunDevelopment merged commit 30b4e25 into PrismJS:master Apr 29, 2020
@RunDevelopment

RunDevelopment commented Apr 29, 2020

Copy link
Copy Markdown
Member

Thank you for contributing @jsmolka!

quentinvernot pushed a commit to TankerHQ/prismjs that referenced this pull request Sep 11, 2020
@jsmolka @quentinvernot
C++: Fixed enum class class names (PrismJS#2342)
be025e3 
The `class` keyword in `enum class` enums was falsely highlighted as the class name.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@RunDevelopment RunDevelopment RunDevelopment approved these changes

Assignees

No one assigned

Labels

enhancement language-definitions needs changes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jsmolka @RunDevelopment