withastro / language-tools

Language tools for Astro
MIT License
272 stars 54 forks source link

Fix `Astro.glob` CodeLens #924

Closed HiDeoo closed 4 months ago

HiDeoo commented 4 months ago

Changes

Following the amazing TBD call you and Ema gave, I discovered the existence of the Astro.glob CodeLens and as it was not working, I decided to fix it.

Before this change, the Astro.glob CodeLens was not being displayed:

SCR-20240730-qozl

After this change, the Astro.glob CodeLens is now displayed:

SCR-20240730-qphq SCR-20240730-qpsq

To give a bit more context to the change, the document.uri in provideCodeLenses() was the one of a Volar embedded document so calling getSourceFile() on the TS program with it was returning undefined. To fix this, I decode the embedded document URI to get the original file URI, and use its fsPath to get the source file and also to get the directory name to get the glob result.

To get the CodeLens to show, I only needed "editor.codeLens": true in my Visual Studio Code settings.

Testing

So this is the weird part as I only tested the change manually. I did not find any existing tests for this (and I assume these would have failed before the change).

I was kinda hoping to be able to get started with something basic like this:

const document = await languageServer.openFakeDocument('---\nawait Astro.glob("../content/docs/**/*.md");\n---', 'astro');
const codeLenses = await languageServer.handle.sendCodeLensesRequest(
  document.uri,
  Position.create(1, 0)
);

Altho, I don't think there is a test utility such as sendCodeLensesRequest() like sendCompletionRequest() for example. Maybe I am totally wrong (my usage of Volar is very limited). Would be happy to add tests if you think it's relevant and if you can guide me on how to do it.

Docs

This is a bug fix only for an undocumented (as far as I know) feature.

Remaining tasks

I did not add a changeset yet as I wanted to get your feedback first and see if the approach I took was correct.

changeset-bot[bot] commented 4 months ago

🦋 Changeset detected

Latest commit: eaeb9e31e68e6aca125632e488cd248231f1bb66

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages | Name | Type | | ------------------------ | ----- | | @astrojs/language-server | Patch | | astro-vscode | Patch |

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Princesseuh commented 4 months ago

Awesome work 😄 Yeah not surprised that Volar paths changes broke this, they changed quite massively in recent versions of Volar, and since there wasn't a test for it, it wasn't caught. It happens.

There's no simple utility for adding a test, as you've noticed, however you can send a request manually in the meantime. I went ahead and added a test for that. We should most definitely add an helper to Volar for that directly (here https://github.com/volarjs/volar.js/blob/master/packages/test-utils/index.ts), but that'll do