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

feat: Support syntax coloration for code block in LSP hover #360

Merged
merged 1 commit into from
Jun 18, 2024

Conversation

angelozerr
Copy link
Contributor

@angelozerr angelozerr commented Jun 14, 2024

Support syntax coloration for code block in LSP hover

Fixes #297

  • Here the result with Lua (the coloration comes from a custom lexer):

image

Do you think it is correct CppCXY?

I copied / pasted some piece of code from QuickDocHighlightingHelper and it seems it is working with Language. For TextMate Language is not enough (because Language is equals to "textmate). I have implemented TextMate support too.

image

  • Here a screenshot with TypeScript TextMate:

image

  • Here a demo with Rust TextMate:

RustHover

  • Here a demo with indented blockquote:

image

@angelozerr angelozerr changed the title Support syntax coloration for code block in LSP hover feat: Support syntax coloration for code block in LSP hover Jun 14, 2024
@angelozerr angelozerr force-pushed the code_block_syntax branch 7 times, most recently from 5210e66 to fbd6310 Compare June 16, 2024 15:45
@fbricon
Copy link
Contributor

fbricon commented Jun 16, 2024

the code block content is duplicated in your screenshot, or you can compare the following
Screenshot 2024-06-17 at 00 29 50

with the JetBrains version:

@angelozerr
Copy link
Contributor Author

the code block content is duplicated in your screenshot, or you can compare the following

Good catch, my translate of kotlin to Java code was bad, it should be fixed now (I have updated screenshot).

@angelozerr angelozerr force-pushed the code_block_syntax branch 3 times, most recently from 635c0f8 to 1c3e275 Compare June 17, 2024 07:45
@angelozerr angelozerr marked this pull request as ready for review June 17, 2024 07:45
Copy link

codecov bot commented Jun 17, 2024

Codecov Report

Attention: Patch coverage is 55.30303% with 118 lines in your changes missing coverage. Please review.

Project coverage is 24.44%. Comparing base (d4d186d) to head (5c4fef8).
Report is 2 commits behind head on main.

Current head 5c4fef8 differs from pull request most recent head 9a8da99

Please upload reports for the commit 9a8da99 to get more accurate results.

Files Patch % Lines
...features/documentation/LSPDocumentationHelper.java 40.00% 29 Missing and 10 partials ⚠️
...documentation/LightQuickDocHighlightingHelper.java 41.46% 18 Missing and 6 partials ⚠️
...cumentation/SyntaxColorationCodeBlockRenderer.java 70.49% 11 Missing and 7 partials ⚠️
...tures/documentation/TextMateHighlighterHelper.java 61.11% 9 Missing and 5 partials ⚠️
...4ij/features/completion/LSPCompletionProposal.java 0.00% 12 Missing ⚠️
...edhat/devtools/lsp4ij/LanguageServersRegistry.java 80.00% 1 Missing and 2 partials ⚠️
...dhat/devtools/lsp4ij/commands/CommandExecutor.java 0.00% 3 Missing ⚠️
...m/redhat/devtools/lsp4ij/ServerMessageHandler.java 0.00% 2 Missing ⚠️
...ols/lsp4ij/launching/ui/ShowInstructionDialog.java 0.00% 2 Missing ⚠️
...hat/devtools/lsp4ij/client/LanguageClientImpl.java 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #360      +/-   ##
==========================================
+ Coverage   22.99%   24.44%   +1.45%     
==========================================
  Files         267      270       +3     
  Lines        9369     9410      +41     
  Branches     1728     1734       +6     
==========================================
+ Hits         2154     2300     +146     
+ Misses       6809     6685     -124     
- Partials      406      425      +19     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jun 17, 2024

Thanks. Glad to know that I'll be able to customize hover support using something more than just a boolean. For comparison, here's how the native API renders the same signature:

Notable differences:

  • None and False have the same style as def, as they are all keywords.
  • Monospace font is used (probably the editor font, but definitely not the UI font), since the entire block is a Markdown code block.
  • Everything is rendered with respect to the current color scheme.

@fbricon
Copy link
Contributor

fbricon commented Jun 17, 2024

Testing this PR's build on Pycharm, with based-pyright enabled:
Screenshot 2024-06-17 at 10 55 25

VS stock Pycharm:

Screenshot 2024-06-17 at 10 55 01

There's a chance the (function) prefix based-pyright adds is disrupting the built-in textmate grammar, causing the style discrepancy with stock pycharm.

As for the missing monospace font, is it a big deal?

@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jun 17, 2024

As for the missing monospace font, is it a big deal?

It is. A Python function may have many parameters, with type hints for each.

Take json.dump() as example:

def dump(
    obj: Any,
    fp: SupportsWrite[str],
    *,
    skipkeys: bool = False,
    ensure_ascii: bool = True,
    check_circular: bool = True,
    allow_nan: bool = True,
    cls: type[JSONEncoder] | None = None,
    indent: None | int | str = None,
    separators: tuple[str, str] | None = None,
    default: Callable[[Any], Any] | None = None,
    sort_keys: bool = False,
    **kwds: Any,
) -> None: ...

Not crazy enough? Check out rich.console.Console.__init__():

def __init__(
    self,
    *,
    color_system: Optional[
        Literal["auto", "standard", "256", "truecolor", "windows"]
    ] = "auto",
    force_terminal: Optional[bool] = None,
    force_jupyter: Optional[bool] = None,
    force_interactive: Optional[bool] = None,
    soft_wrap: bool = False,
    theme: Optional[Theme] = None,
    stderr: bool = False,
    file: Optional[IO[str]] = None,
    quiet: bool = False,
    width: Optional[int] = None,
    height: Optional[int] = None,
    style: Optional[StyleType] = None,
    no_color: Optional[bool] = None,
    tab_size: int = 8,
    record: bool = False,
    markup: bool = True,
    emoji: bool = True,
    emoji_variant: Optional[EmojiVariant] = None,
    highlight: bool = True,
    log_time: bool = True,
    log_path: bool = True,
    log_time_format: Union[str, FormatTimeCallable] = "[%X]",
    highlighter: Optional["HighlighterType"] = ReprHighlighter(),
    legacy_windows: Optional[bool] = None,
    safe_box: bool = True,
    get_datetime: Optional[Callable[[], datetime]] = None,
    get_time: Optional[Callable[[], float]] = None,
    _environ: Optional[Mapping[str, str]] = None,
):

For the main part, these signatures are code, and (unless you are into that kind of thing) code are meant to be displayed in monospace font.

@fbricon
Copy link
Contributor

fbricon commented Jun 17, 2024

it's still readable
Screenshot 2024-06-17 at 11 34 44

@InSyncWithFoo
Copy link
Contributor

InSyncWithFoo commented Jun 17, 2024

I, for one, largely prefer it this way, and I think dyslexic people would agree:

Readability notwithstanding, here's how the block is sent via LSP (pseudo-JSON):

{
  "result": {
    "contents": {
      "kind": "markdown",
      "value": """
        ```python
        (function) def dumps(
            obj: Any,
            *,
            skipkeys: bool = False,
            ...
        ) -> str
        ```
        ---
        Serialize `obj` to a JSON formatted `str`.
        
        If `skipkeys` is true then `dict` keys that are not basic types [...]
      """
    },
    "range": {/* ... */}
}

That message should, obviously, be rendered as one code block followed by a horizontal ruler and multiple paragraphs:

(function) def dumps(
    obj: Any,
    *,
    skipkeys: bool = False,
    ...
) -> str

Serialize obj to a JSON formatted str.

If skipkeys is true then dict keys that are not basic types [...]

@fbricon
Copy link
Contributor

fbricon commented Jun 17, 2024

There you go:

Screenshot 2024-06-17 at 12 01 39

just needed to wrap the code rendering with <pre>

@angelozerr angelozerr force-pushed the code_block_syntax branch 7 times, most recently from ec7985c to 92a1e4a Compare June 18, 2024 05:36
@angelozerr
Copy link
Contributor Author

angelozerr commented Jun 18, 2024

@fbricon if you want to test quickly this PR, you can create a ts file with documentation like:

/**

ts: const s= '';


```xml
xml: <foo></foo>
 java: String s = new String("abed");
  > const s = "";
def dump(
    obj: Any,
    fp: SupportsWrite[str],
    *,
    skipkeys: bool = False,
    ensure_ascii: bool = True,
    check_circular: bool = True,
    allow_nan: bool = True,
    cls: type[JSONEncoder] | None = None,
    indent: None | int | str = None,
    separators: tuple[str, str] | None = None,
    default: Callable[[Any], Any] | None = None,
    sort_keys: bool = False,
    **kwds: Any,
) -> None: ...

**/

function foo() {
}

foo();

and hover the foo:

image

} else if (content.isRight()) {
MarkedString markedString = content.getRight();
if (!isValidContent(markedString)) {
return null;
Copy link
Contributor

Choose a reason for hiding this comment

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

return should be @NotNull

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It should, because it is filtered after.

@angelozerr angelozerr force-pushed the code_block_syntax branch 2 times, most recently from e04f184 to 5c4fef8 Compare June 18, 2024 15:56
@fbricon
Copy link
Contributor

fbricon commented Jun 18, 2024

Thanks @angelozerr!

@fbricon fbricon merged commit 71f6d73 into redhat-developer:main Jun 18, 2024
6 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.

Support syntax coloration for code block in LSP hover
3 participants