Skip to content

Conversation

@JoeRobich
Copy link
Member

@JoeRobich JoeRobich commented Oct 3, 2025

image

@JoeRobich JoeRobich force-pushed the dev/jorobich/app-directives branch from a655f40 to 8398aed Compare October 3, 2025 23:14
'1': { name: punctuation.separator.colon.cs }
end: (?=$)
patterns:
- include: '#preprocessor-app-directive-package'
Copy link
Member

Choose a reason for hiding this comment

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

I'm torn between telling you to go add sdk support as well (and ask @jjonescz what other directives are supported), or just saying that we shouldn't do any of these and just do the generic version. Your call.

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure what you mean, sdk seems to be supported here? I think you got all the directives we currently support.

Copy link
Member

Choose a reason for hiding this comment

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

Apparently my eyes glazed over the last section... that's what happens when you review PRs at 10:30 PM I guess.

Comment on lines +720 to +722
Token.Identifier.PreprocessorSymbol("Foo"),
Token.Punctuation.Dot,
Token.Identifier.PreprocessorSymbol("Goo"),
Copy link
Member

@RikkiGibson RikkiGibson Oct 4, 2025

Choose a reason for hiding this comment

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

Was there a specific reason to make the dots separate tokens? It doesn't look like this is how preprocessor symbols are handled, for example:

Image

wait.. no. maybe it is. It's just in my theme I have to zoom in to be able to distinguish the color of the dot.

Copy link
Member Author

Choose a reason for hiding this comment

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

We certainly don't have to, but many packages are dot separated and I thought it might be a good visual signal.

@JoeRobich JoeRobich merged commit fdd598d into main Oct 6, 2025
2 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.

5 participants