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

Initial implementation of nimsuggest v3 (#19826) [backport] #19892

Merged
merged 3 commits into from
Jun 19, 2022

Conversation

yyoncho
Copy link
Contributor

@yyoncho yyoncho commented Jun 13, 2022

  • Initial implementation of nimsuggest v3

Rework nimsuggest to use caching to make usage of ide commands more efficient.
Previously, all commands no matter what the state of the process is were causing
clean build. In the context of Language Server Protocol(LSP) and lsp clients
this was causing perf issues and overall instability. Overall, the goal of v3 is
to fit to LSP Server needs

  • added two new commands:

    • recompile to do clean compilation
    • changed which can be used by the IDEs to notify that a particular file has been changed.
      The later can be utilized when using LSP file watches.
    • globalSymbols - searching global references
  • added segfaults dependency to allow fallback to clean build when incremental
    fails. I wish the error to be propagated to the client so we can work on fixing
    the incremental build failures (typically hitting pointer)

  • more efficient rebuild flow. ATM incremental rebuild is triggered when the
    command needs that(i. e. it is global) while the commands that work on the
    current source rebuild only it

Things missing in this PR:

  • Documentation
  • Extensive unit testing.

Although functional I still see this more as a POC that this approach can work.

Next steps:

  • Implement sug request.
  • Rework/extend the protocol to allow better client/server communication.
    Ideally we will need push events, diagnostics should be restructored to allow
    per file notifications, etc.
  • implement v3 test suite.
  • better logging
  • Add tests for v3 and implement ideSug

  • Remove typeInstCache/procInstCache cleanup

  • Add ideChkFile command

  • Avoid contains call when adding symbol info

  • Remove log

  • Remove segfaults

yyoncho and others added 3 commits June 13, 2022 12:42
* Initial implementation of nimsuggest v3

Rework `nimsuggest` to use caching to make usage of ide commands more efficient.
Previously, all commands no matter what the state of the process is were causing
clean build. In the context of Language Server Protocol(LSP) and lsp clients
this was causing perf issues and overall instability. Overall, the goal of v3 is
to fit to LSP Server needs

- added two new commands:
  - `recompile` to do clean compilation
  - `changed` which can be used by the IDEs to notify that a particular file has been changed.
The later can be utilized when using LSP file watches.
  - `globalSymbols` - searching global references

- added `segfaults` dependency to allow fallback to clean build when incremental
fails. I wish the error to be propagated to the client so we can work on fixing
the incremental build failures (typically hitting pointer)

- more efficient rebuild flow. ATM incremental rebuild is triggered when the
command needs that(i. e. it is global) while the commands that work on the
current source rebuild only it

Things missing in this PR:

- Documentation
- Extensive unit testing.

Although functional I still see this more as a POC that this approach can work.

Next steps:
- Implement `sug` request.
- Rework/extend the protocol to allow better client/server communication.
Ideally we will need push events, diagnostics should be restructored to allow
per file notifications, etc.
- implement v3 test suite.
- better logging

* Add tests for v3 and implement ideSug

* Remove typeInstCache/procInstCache cleanup

* Add ideChkFile command

* Avoid contains call when adding symbol info

* Remove log

* Remove segfaults
- make sure transitive deps are marked as dirty
@Araq Araq merged commit ab0d068 into nim-lang:version-1-6 Jun 19, 2022
EmilIvanichkovv pushed a commit to metacraft-labs/nim that referenced this pull request Aug 19, 2022
…im-lang#19892)

* Initial implementation of nimsuggest v3 (nim-lang#19826)

* Initial implementation of nimsuggest v3

Rework `nimsuggest` to use caching to make usage of ide commands more efficient.
Previously, all commands no matter what the state of the process is were causing
clean build. In the context of Language Server Protocol(LSP) and lsp clients
this was causing perf issues and overall instability. Overall, the goal of v3 is
to fit to LSP Server needs

- added two new commands:
  - `recompile` to do clean compilation
  - `changed` which can be used by the IDEs to notify that a particular file has been changed.
The later can be utilized when using LSP file watches.
  - `globalSymbols` - searching global references

- added `segfaults` dependency to allow fallback to clean build when incremental
fails. I wish the error to be propagated to the client so we can work on fixing
the incremental build failures (typically hitting pointer)

- more efficient rebuild flow. ATM incremental rebuild is triggered when the
command needs that(i. e. it is global) while the commands that work on the
current source rebuild only it

Things missing in this PR:

- Documentation
- Extensive unit testing.

Although functional I still see this more as a POC that this approach can work.

Next steps:
- Implement `sug` request.
- Rework/extend the protocol to allow better client/server communication.
Ideally we will need push events, diagnostics should be restructored to allow
per file notifications, etc.
- implement v3 test suite.
- better logging

* Add tests for v3 and implement ideSug

* Remove typeInstCache/procInstCache cleanup

* Add ideChkFile command

* Avoid contains call when adding symbol info

* Remove log

* Remove segfaults

* Fixed bad cherry-pick resolve

* modulegraphs.dependsOn does not work on transitive modules

- make sure transitive deps are marked as dirty
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