Skip to content

Refactor extension#407

Merged
doriable merged 41 commits intomainfrom
refactor-extension-state
Jul 25, 2025
Merged

Refactor extension#407
doriable merged 41 commits intomainfrom
refactor-extension-state

Conversation

@doriable
Copy link
Member

@doriable doriable commented Jul 15, 2025

This is a refactor of the LSP implementation for the extension:

  • Rename BufContext -> BufState, export as a global, and centralise extension logic in BufState: rather than context, this is more of a state for the extension itself. It was previously instantiated and plumbed through all commands, but since the state is global, it made sense to change the pattern and export it was a global variable that could be imported wherever it was needed. Most of the extension state logic (e.g. managing the buf binary and the language server) have also been centralised to BufState. Commands are now simple functions exposed to the user that call functions on BufState.
  • The refactor of BufContext simplified the Command structure.
  • Consolidate find-buf and install-buf: find-buf and install-buf were closely coupled, and we wanted to expose a clear command to our users in the form of an "install" command. Extension activation uses "install" as well.
  • Rather than restart-buf and stop-buf, we now have startLanguageServer and stopLanguageServer: this is a small semantic change, but it helps the commands map closer to the language server states, LANGUAGE_SERVER_STARTING and LANGUAGE_SERVER_STOPPED. Also those commands only control LSP functionality using buf.
  • Use string[] and as const as literal value types instead of enums: this is just a pattern that was preferred, and allows us to export/import less types.
  • Revamp testing: we wanted to do as little mocking/stubbing as possible in our tests, and we generally want to test integration flows. The testing has been reworked and documented in DEVELOPMENT.md. All tests are either using the VS Code testing CLI or Playwright. Also added testing for LSP functionality in Playwright.
  • Clean-up .vscodeignore to reduce the number of files we were including that were not needed at runtime.

There will be a follow-up PR containing a new CHANGELOG and README. We'll also be updating our lint + deps in a follow-up PR.

@doriable doriable requested a review from paul-sachs July 15, 2025 03:52
@doriable doriable requested a review from paul-sachs July 21, 2025 22:08
*
* COMMAND_TYPE_BUF commands are used to execute non-LSP server Buf CLI commands, e.g. `buf lint`.
*/
type CommandType = (typeof _commandType)[number];
Copy link
Member

Choose a reason for hiding this comment

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

Care to explain why we're doing this typeof nonsense? I expect non-TS experts to have to look at this, so it'd be best to explain anything especially tricky looking (such as not using an enum).

Copy link
Contributor

Choose a reason for hiding this comment

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

Basically to avoid enums. Enums generate some pretty horrendous javascript, and iterating over them is confusing at best (and erroneous at worst). This bypasses that by using string literals instead and let's typescript enforce validity that way. A slight sacrifice to that is this typescript logic to get the possible values of that array.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added a comment in status.ts and command.ts explaining basically what Paul communicated above so that we have some in-line documentation for why we do this.

@doriable doriable merged commit 147f749 into main Jul 25, 2025
5 checks passed
@doriable doriable deleted the refactor-extension-state branch July 25, 2025 18:03
stefanvanburen added a commit that referenced this pull request Jan 23, 2026
I think using `.path` for these is causing the issues we're seeing in
#522 with being unable to run these commands on windows, as we're
passing in a `slash/separated/path` for the `cwd`, resulting in ENOENTs.

We originally reworked these commands in #407, for reference.

Fixes #522.
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.

3 participants