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

Implicit token highlighting #383

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

fan-tom
Copy link

@fan-tom fan-tom commented Nov 1, 2024

Motivation

Currently both implicit and explicit tokens (terminology from here) share a common highlighting key: BNF_TOKEN, which makes it impossible to assign different highlighting styles to them and spot issues in the grammar, when e.g. because of a typo implicit token is used instead of the name of an explicit one.
E.g. this grammar

{
  tokens = [
    OPEN_PARENTHESE = '('
    CLOSE_PARENTHESE = ')'
    ID = "regexp:\w+"
  ]
}

file ::= OPEN_PARENTHES ID+ CLOSE_PARENTHESE

Is highlighted like this
изображение

This PR makes it to be highlighted like this (by default, may be overridden on the color settings page)
изображение
which lets developer notice that something's off, and overall distinguish explicit tokens, matched by name and implicitly defined tokens, the same way we already distinguish explicit tokens, matched by value and toekns, matched by text (called "patterns" in the code base)

Additional changes

There are 2 sets of commits:

  • those which introduce separate a highlighting for implicit tokens, with minumum changes across the codebase
  • those I considered to be nice to have (they have fix: prefix):
    • let IDEA highlight bnf example text on the color settings page as an HTML
    • use more of instanceof pattern matching syntax where I already started using it
    • rename TOKEN to EXPLICIT_TOKEN to make it symmetric with IMPLICIT_TOKEN

Please advice which of them are desired and which I must drop

@YannCebron YannCebron self-requested a review January 16, 2025 15:42
Copy link
Member

@YannCebron YannCebron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this!

@@ -27,7 +27,8 @@ class BnfSyntaxHighlighter extends SyntaxHighlighterBase {
public static final TextAttributesKey PATTERN = createTextAttributesKey("BNF_PATTERN", DefaultLanguageHighlighterColors.INSTANCE_FIELD);
public static final TextAttributesKey NUMBER = createTextAttributesKey("BNF_NUMBER", DefaultLanguageHighlighterColors.NUMBER);
public static final TextAttributesKey KEYWORD = createTextAttributesKey("BNF_KEYWORD", DefaultLanguageHighlighterColors.MARKUP_ATTRIBUTE);
public static final TextAttributesKey TOKEN = createTextAttributesKey("BNF_TOKEN", DefaultLanguageHighlighterColors.STRING);
public static final TextAttributesKey EXPLICIT_TOKEN = createTextAttributesKey("BNF_EXPLICIT_TOKEN", DefaultLanguageHighlighterColors.STRING);
Copy link
Member

Choose a reason for hiding this comment

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

changing the identifier will break any customizations users already have, please keep "BNF_TOKEN" as ID

Copy link
Author

Choose a reason for hiding this comment

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

fixed

@fan-tom fan-tom force-pushed the implicit_token_highlighting branch from e065303 to 491ddbf Compare January 16, 2025 22:47
@fan-tom fan-tom requested a review from YannCebron January 16, 2025 22:59
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.

2 participants