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

Document which files the language client should watch #11

Merged
merged 3 commits into from
Sep 25, 2023
Merged

Document which files the language client should watch #11

merged 3 commits into from
Sep 25, 2023

Conversation

remcohaszing
Copy link
Member

Initial checklist

  • I read the support docs
  • I read the contributing guide
  • I agree to follow the code of conduct
  • I searched issues and couldn’t find anything (or linked relevant results below)
  • If applicable, I’ve added docs and tests

Description of changes

Document language client should watch remark configuration files.

@remcohaszing remcohaszing added 📚 area/docs This affects documentation 🤞 phase/open Post is being triaged manually labels Aug 26, 2023
@@ -50,6 +50,7 @@ Practical examples are coming soon.

`remark-language-server` uses the same configuration files as
[`remark-cli`][remark-cli].
Language clients should notify the language server if these files change.
Copy link
Member Author

Choose a reason for hiding this comment

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

It may be good to make an exact list of these files, ideally by putting this in a dedicated section in https://github.com/remarkjs/remark/blob/main/packages/remark-cli/readme.md.

Copy link
Member

Choose a reason for hiding this comment

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

it says that right above examples? https://github.com/remarkjs/remark/blob/main/packages/remark-cli/readme.md#examples Or what are you thinking off?

I still don’t get what I’d have to if I were to use this. “These files” is vague to me. Should particular paths be watched? Or basenames on an entire file system? Do I need to set up chokidar or something? unified-engine is going to walk “outside” of VSC workspaces. Should say ~/.remarkrc also be watched? Is it clear how to notify this language server of changes?

Copy link
Member Author

Choose a reason for hiding this comment

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

it says that right above examples? https://github.com/remarkjs/remark/blob/main/packages/remark-cli/readme.md#examples Or what are you thinking off?

I think this demonstrates the problem. You link a section, then tell me to scroll up. It would be nice if we could link to https://github.com/remarkjs/remark/blob/main/packages/remark-cli/readme.md#configuration instead.

Above https://github.com/remarkjs/remark/blob/main/packages/remark-cli/readme.md#compatibility is a full list of configuration files. I think it’s good to move this full list into the Configuration section I just proposed. It’s an important part of remark-cli for end users to unserstand, so I think if this information is easily accessible without any form of redirection to an example of unified-engine.

This also makes me think it’s probably best to duplicate the list here in remark-language-server. It’s not likely to change anyway.

Do I need to set up chokidar or something? […] Is it clear how to notify this language server of changes?

The language client needs to setup a file watcher. Whatever this means depends on the language client library. How this works should generally be clear for people working with their library.

unified-engine is going to walk “outside” of VSC workspaces. Should say ~/.remarkrc also be watched?

This is a good question. I will mention this, but leave the decision up to the implementator.

Copy link
Member

Choose a reason for hiding this comment

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

I mean, it’s in the docs, I tried to link as close as can be 🤷‍♂️

The downside w adding a whole section on how config files works in remark-cli is that we had that. And we have similar “problems” elsewhere. It means duplicating content from lower-level projects upwards.
I’m always open to improving this, but that’s an important trade off. Should things be copy/pasted, or short in higher projects and explained in detail in the projects that actually do the work?

As for this OG comment, feel free to PR to improve the remark-cli readme!

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 think this list is now fine as-is. The most important goal here is to list which files need to be watched for projects that implement the remark-language-server.

@remcohaszing remcohaszing changed the title Document language client should watch config files Document which files the language client should watch Sep 25, 2023
@remcohaszing remcohaszing merged commit 3ae00d5 into remarkjs:main Sep 25, 2023
3 checks passed
@remcohaszing remcohaszing deleted the document-watched-files branch September 25, 2023 16:11
@github-actions
Copy link

Hi! This was closed. Team: If this was merged, please describe when this is likely to be released. Otherwise, please add one of the no/* labels.

@remcohaszing remcohaszing added 👍 phase/yes Post is accepted and can be worked on 🙆 yes/confirmed This is confirmed and ready to be worked on labels Sep 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 area/docs This affects documentation 🤞 phase/open Post is being triaged manually 👍 phase/yes Post is accepted and can be worked on 🙆 yes/confirmed This is confirmed and ready to be worked on
Development

Successfully merging this pull request may close these issues.

None yet

2 participants