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

Inlay hints support #22896

Merged
merged 15 commits into from
Nov 4, 2023
Merged

Inlay hints support #22896

merged 15 commits into from
Nov 4, 2023

Conversation

nickysn
Copy link
Contributor

@nickysn nickysn commented Oct 31, 2023

This adds inlay hints support to nimsuggest. It adds a new command to nimsuggest, called 'inlayHints'.

Currently, it provides type information to 'var' and 'let' variables. In the future, inlay hints can also be added for 'const' and for function parameters. The protocol also reserves space for a tooltip field, which is not used, yet, but support for it can be added in the future, without further changing the protocol.

The change includes refactoring to allow the 'inlayHints' command to return a completely different structure, compared to the other nimsuggest commands. This will allow other future commands to have custom return types as well. All the previous commands return the same structure as before, so perfect backwards compatibility is maintained.

To use this feature, an update to the nim language server, as well as the VS code extension is needed.

Related PRs:
nimlangserver: nim-lang/langserver#53
VS code extension: saem/vscode-nim#134

…d to the

suggestSym proc. Only show inlay suggest hints for symbols with isDecl = true.
…fied

explicitly. We use this to avoid returning inlay type hints for vars that have
already a type, specified in code.
… future

use, without changing the nimsuggest protocol for the result of the inlayHints
command)
@nickysn
Copy link
Contributor Author

nickysn commented Oct 31, 2023

CI failure is because nimlsp uses structures from the compiler sources. Suggest is now a base class, with multiple descendants, and all the members of what was previously Suggest are now in SuggestDef. Nimlsp needs to be updated in order to compile against the new compiler sources. Note that there's no change in the nimsuggest protocol, so already compiled versions of nimlsp should work with the new nimsuggest.

@Araq
Copy link
Member

Araq commented Oct 31, 2023

I assume it's easier to keep backwards compatibility when you do away with the newly introduced inheritance? The added fields are nothing to worry about anyway.

…order to

keep compatiblity with existing nimsuggest clients, that use the nimsuggest
structures from the compiler sources as a library.
… when the position is in the first or last line)
…, because

Nim doesn't support declaring a variable type for 'for' loops
@nickysn
Copy link
Contributor Author

nickysn commented Nov 2, 2023

Ok, got rid of the inheritance. The remaining CI failure looks like a random glitch and unrelated to my changes.

@@ -105,7 +105,7 @@ proc getTokenLenFromSource(conf: ConfigRef; ident: string; info: TLineInfo): int
result = 0
elif ident[0] in linter.Letters and ident[^1] != '=':
result = identLen(line, column)
if cmpIgnoreStyle(line[column..column + result - 1], ident) != 0:
if cmpIgnoreStyle(line[column..column + result - 1], ident[0..min(result-1,len(ident)-1)]) != 0:
Copy link
Member

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because ident may have extra stuff appended when part of a template. E.g. when a variable is part of a template, "`gensymXXX" is appended, where XXX are some numbers. In the previous version of the code, this would return a token length of 0. In turn, this causes the inlay hint to be put in the wrong place. Instead of:

template benchmark(benchmarkName: string, code: untyped) =
  block:
    debug "Started...", benchmark = benchmarkName
    let t0: float = epochTime()
    code
    let elapsed: float = epochTime() - t0
    let elapsedStr: string = elapsed.formatFloat(format = ffDecimal, precision = 3)
    debug "CPU Time", benchmark = benchmarkName, time = elapsedStr

You would get:

template benchmark(benchmarkName: string, code: untyped) =
  block:
    debug "Started...", benchmark = benchmarkName
    let : floatt0 = epochTime()
    code
    let : floatelapsed = epochTime() - t0
    let : stringelapsedStr = elapsed.formatFloat(format = ffDecimal, precision = 3)
    debug "CPU Time", benchmark = benchmarkName, time = elapsedStr

Copy link
Member

Choose a reason for hiding this comment

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

Sucks, but ok.

compiler/suggest.nim Outdated Show resolved Hide resolved
compiler/suggest.nim Outdated Show resolved Hide resolved
nimsuggest/nimsuggest.nim Outdated Show resolved Hide resolved
@Araq Araq merged commit 3f2b9c8 into nim-lang:devel Nov 4, 2023
19 checks passed
Copy link
Contributor

github-actions bot commented Nov 4, 2023

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 3f2b9c8

Hint: mm: orc; opt: speed; options: -d:release
176413 lines; 7.168s; 767.852MiB peakmem

@nickysn nickysn deleted the inlay_hints_support branch November 6, 2023 11:34
nickysn added a commit to nickysn/Nim that referenced this pull request Nov 7, 2023
This adds inlay hints support to nimsuggest. It adds a new command to
nimsuggest, called 'inlayHints'.

Currently, it provides type information to 'var' and 'let' variables. In
the future, inlay hints can also be added for 'const' and for function
parameters. The protocol also reserves space for a tooltip field, which
is not used, yet, but support for it can be added in the future, without
further changing the protocol.

The change includes refactoring to allow the 'inlayHints' command to
return a completely different structure, compared to the other
nimsuggest commands. This will allow other future commands to have
custom return types as well. All the previous commands return the same
structure as before, so perfect backwards compatibility is maintained.

To use this feature, an update to the nim language server, as well as
the VS code extension is needed.

Related PRs:
nimlangserver: nim-lang/langserver#53
VS code extension: saem/vscode-nim#134

---------

Co-authored-by: Andreas Rumpf <[email protected]>
(cherry picked from commit 3f2b9c8)
nickysn added a commit to nickysn/Nim that referenced this pull request Nov 7, 2023
This adds inlay hints support to nimsuggest. It adds a new command to
nimsuggest, called 'inlayHints'.

Currently, it provides type information to 'var' and 'let' variables. In
the future, inlay hints can also be added for 'const' and for function
parameters. The protocol also reserves space for a tooltip field, which
is not used, yet, but support for it can be added in the future, without
further changing the protocol.

The change includes refactoring to allow the 'inlayHints' command to
return a completely different structure, compared to the other
nimsuggest commands. This will allow other future commands to have
custom return types as well. All the previous commands return the same
structure as before, so perfect backwards compatibility is maintained.

To use this feature, an update to the nim language server, as well as
the VS code extension is needed.

Related PRs:
nimlangserver: nim-lang/langserver#53
VS code extension: saem/vscode-nim#134

---------

Co-authored-by: Andreas Rumpf <[email protected]>
(cherry picked from commit 3f2b9c8)
nickysn added a commit to nickysn/Nim that referenced this pull request Nov 8, 2023
This adds inlay hints support to nimsuggest. It adds a new command to
nimsuggest, called 'inlayHints'.

Currently, it provides type information to 'var' and 'let' variables. In
the future, inlay hints can also be added for 'const' and for function
parameters. The protocol also reserves space for a tooltip field, which
is not used, yet, but support for it can be added in the future, without
further changing the protocol.

The change includes refactoring to allow the 'inlayHints' command to
return a completely different structure, compared to the other
nimsuggest commands. This will allow other future commands to have
custom return types as well. All the previous commands return the same
structure as before, so perfect backwards compatibility is maintained.

To use this feature, an update to the nim language server, as well as
the VS code extension is needed.

Related PRs:
nimlangserver: nim-lang/langserver#53
VS code extension: saem/vscode-nim#134

---------

Co-authored-by: Andreas Rumpf <[email protected]>
(cherry picked from commit 3f2b9c8)
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