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

Add API to syntax-highlight code #386

Closed
rock3r opened this issue May 16, 2024 · 5 comments · Fixed by #592
Closed

Add API to syntax-highlight code #386

rock3r opened this issue May 16, 2024 · 5 comments · Fixed by #592
Assignees
Labels
feature New feature or request markdown This issue impacts the Markdown rendering subsystem

Comments

@rock3r
Copy link
Collaborator

rock3r commented May 16, 2024

Users may want to display code, and having syntax highlighting is important for readability.

We need an API that can be used to turn a string of code into a highlighted AnnotatedString. On the bridge side, the styling must come from the EditorScheme, and on the standalone side, we should use the defaults for Light and Dark themes. On the bridge side, we expect fully semantic highlighting where supported by the current IDE, and on the standalone side, we expect basic syntax-based highlighting. On the bridge side, all the languages supported by the current IDE must be supported, and on the standalone side we must support at a minimum Kotlin, Java, XML, JSON, JS, TS, HTML, CSS, Python. Other languages support is nice to have but not required.

Thoughts:

  1. Highlighting depends on the EditorScheme in bridge, and theme in standalone. If it is possible to avoid re-computing the highlighing info (e.g., determining code semantics) but limit ourselves to creating a new AnnotatedString with updated spans, this would be preferable for perf reasons.
  2. The styling scheme should be stored in a composition local, and accessible via a theme-level getter, such as JewelTheme.codeStyling
  3. Styling should follow the usual pattern of "definition in foundation, implementations in standalone and bridge"
@rock3r rock3r added the feature New feature or request label May 16, 2024
@ghost ghost added the markdown This issue impacts the Markdown rendering subsystem label Jul 25, 2024
@francisconoriega
Copy link
Contributor

francisconoriega commented Aug 28, 2024

@rock3r I created PR #565 and was about to open the corresponding GH Issue for it, but I see its already here :)

That PR mostly follows what you have defined here, except for the the theme-level getter.

Thoughts on the implementation / use of the 3rd party library?

@AlexVanGogen
Copy link
Collaborator

Hi @francisconoriega! Hope you don't mind me jumping in, I was looking at the issue recently and gotten a couple thoughts.

Editor scheme in the IDE allows specifying different colors for the same token for different languages. For example, set orange for keywords in Java and blue in Kotlin. Implementors might also want to highlight identifiers (even more so, different identifiers with different colors), underline syntax errors and so on.

It would be nice if Jewel API could reflect all that. Which means highlighter should also be a composition local, because all this extravaganza might be a bit too much for standalone. Something like this:

public interface MarkdownCodeHighligher {
    public /* suspend? */ fun highlight(code: String, mimeType: MimeType /* , codeBlock: MarkdownBlock.Code.Fenced */): AnnotatedString
}

i.e. pretty much how it's described in the issue 😄

It gives a lot of flexibility to implementors, and leaves room for optimizations. Like, cache lexer/parser result using MarkdownBlock as a key so that parsing could be skipped if text remained the same, or something like that.

And I think foundation should be kept as simple as possible. Probably there shouldn't be any highlighting logic (and an additional dependency that comes with it) at all. A 3rd party library is worth using in standalone version, whereas in bridge we could just use IntelliJ SDK.

@rock3r
Copy link
Collaborator Author

rock3r commented Aug 29, 2024

Another note — I think syntax highlighting is not necessarily something we want to tie exclusively to the Markdown renderer. I can see the need for it elsewhere too

@francisconoriega
Copy link
Contributor

hmm all fair points.

@rock3r do you want to go with a different approach (ie I should stop work on the current PR), or do feel okay going this way, if the comments in the PR are addressed?

@rock3r
Copy link
Collaborator Author

rock3r commented Aug 29, 2024

I feel like we should probably have a chat before you spend more time on this, so we are better aligned. Can jump on Slack in a few minutes?

@rock3r rock3r added the in-progress This issue is being worked on label Sep 6, 2024
AlexVanGogen added a commit to AlexVanGogen/jewel that referenced this issue Sep 10, 2024
* Provide default no-op highlighter
* Use highlighter to render code in `DefaultMarkdownBlockRenderer`
* Extract `MimeType` from markdown to core
@rock3r rock3r linked a pull request Sep 10, 2024 that will close this issue
@rock3r rock3r linked a pull request Sep 10, 2024 that will close this issue
AlexVanGogen added a commit to AlexVanGogen/jewel that referenced this issue Sep 20, 2024
* Provide default no-op highlighter
* Use highlighter to render code in `DefaultMarkdownBlockRenderer`
* Extract `MimeType` from markdown to core
rock3r pushed a commit that referenced this issue Sep 23, 2024
* Introduce code highlighting API (#386)

* Provide default no-op highlighter
* Use highlighter to render code in `DefaultMarkdownBlockRenderer`
* Extract `MimeType` from markdown to core

* Provide highlighter implementations

* Use no-op in standalone
* Use IJP lexer-based highlighter in IDE bridge; no-op as default
    * otherwise we need to pass the project around
* Provide option to define own highlighter via `ProvideMarkdownStyling`

* Re-highlight code when the editor scheme color changed

* API dump

* Inject highlighting dispatcher into the constructor

* Add `ProvideMarkdownStyling` override with the project

Using this override will automatically set up a highlighter,
no need to do that manually.

* Fail fast if file type is unknown

* Background and text decoration in TextAttributes -> SpanStyle conversion

* Revert MimeType looks

* Simplify NoOpCodeHighlighter

* Remove unnecessary color specifier
rock3r pushed a commit that referenced this issue Sep 24, 2024
* Introduce code highlighting API (#386)

* Provide default no-op highlighter
* Use highlighter to render code in `DefaultMarkdownBlockRenderer`
* Extract `MimeType` from markdown to core

* Provide highlighter implementations

* Use no-op in standalone
* Use IJP lexer-based highlighter in IDE bridge; no-op as default
    * otherwise we need to pass the project around
* Provide option to define own highlighter via `ProvideMarkdownStyling`

* Re-highlight code when the editor scheme color changed

* API dump

* Inject highlighting dispatcher into the constructor

* Add `ProvideMarkdownStyling` override with the project

Using this override will automatically set up a highlighter,
no need to do that manually.

* Fail fast if file type is unknown

* Background and text decoration in TextAttributes -> SpanStyle conversion

* Revert MimeType looks

* Simplify NoOpCodeHighlighter

* Remove unnecessary color specifier
@rock3r rock3r removed the in-progress This issue is being worked on label Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request markdown This issue impacts the Markdown rendering subsystem
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants