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

file.cwd is not correctly set #20

Closed
4 tasks done
wooorm opened this issue Dec 31, 2021 · 5 comments · Fixed by #22
Closed
4 tasks done

file.cwd is not correctly set #20

wooorm opened this issue Dec 31, 2021 · 5 comments · Fixed by #22
Labels
💪 phase/solved Post is done

Comments

@wooorm
Copy link
Member

wooorm commented Dec 31, 2021

Initial checklist

Affected packages and versions

latest

Link to runnable example

No response

Steps to reproduce

cwd is not set when creating a file or when creating an engine.

file.cwd must be set to the folder that is the “workplace”, e.g., the place where the package.json and .git are typically.
Right now, it’s not set, so it’s (90% sure) set to a wrong value. Which means certain plugins break (example: https://github.com/remarkjs/remark-usage/blob/ae591a38bfb5424a802f25ae77b69f5dc06836fa/lib/index.js#L13-L27)

Expected behavior

Here’s how I found a logical one for atom: https://github.com/unifiedjs/unified-engine-atom/blob/605326d08a6d04b66aa8abf3143313f24c507699/index.js#L44-L53

Actual behavior

(unknown behavior, likely broken plugins or unexpected results)

Runtime

No response

Package manager

No response

OS

No response

Build and bundle tools

No response

@github-actions github-actions bot added 👋 phase/new Post is being triaged automatically 🤞 phase/open Post is being triaged manually and removed 👋 phase/new Post is being triaged automatically labels Dec 31, 2021
@remcohaszing
Copy link
Member

Isn’t it set to process.cwd() by default? Typically the client determines the cwd.

Also does it make sense if unified-engine detects this? I imagine a good cwd to set would be the directory containing the configuration file.

@wooorm
Copy link
Member Author

wooorm commented Jan 2, 2022

Yes, but the value provided by process.cwd does not always make sense.
Atom sets it to / (which is why remark-parse can’t be resolved and why linter-remark doesn’t work with the PR)
I’m also guessing the Node.js language server runs in a cwd that is different from the “workspace”, as they‘re separate processed?

unified-engine has an option for it: options.cwd, which can be given the CWD. This can (should?) be set too.

I imagine a good cwd to set would be the directory containing the configuration file.

Files are found first, configuration files are found from them. That means that there can be a) multiple configuration files, b) none, c) one in say, /Users/

@ChristianMurphy
Copy link
Member

Maybe the workspace folders(s) could be used? https://microsoft.github.io/language-server-protocol/specifications/specification-3-17/#workspace_workspaceFolders

@wooorm
Copy link
Member Author

wooorm commented Jan 2, 2022

Nice that LSP also has that. For Atom, the earlier linked lines (https://github.com/unifiedjs/unified-engine-atom/blob/605326d08a6d04b66aa8abf3143313f24c507699/index.js#L44-L53) uses something like that too.

@remcohaszing
Copy link
Member

Yes, I think using rootUri of the InitializeParams should be used.

wooorm added a commit that referenced this issue Jan 7, 2022
This adds support for workspaces, which in most cases will be just a simple
single workspace (sort of like a working directory on the terminal), but in
more complex cases can be multiple workspaces and which change over time.

The closest open workspace is chosen when processing a file, in batches of
files that belong together in a workspace.

For backwards compatibility with LSP 3.6.0, `event.rootUri` is also supported.

Closes GH-20.
@wooorm wooorm mentioned this issue Jan 7, 2022
5 tasks
wooorm added a commit that referenced this issue Jan 8, 2022
This adds support for workspaces, which in most cases will be just a simple
single workspace (sort of like a working directory on the terminal), but in
more complex cases can be multiple workspaces and which change over time.

The closest open workspace is chosen when processing a file, in batches of
files that belong together in a workspace.

For backwards compatibility with LSP 3.6.0, `event.rootUri` is also supported.

Closes GH-20.
@wooorm wooorm added the 💪 phase/solved Post is done label Jan 10, 2022
@github-actions github-actions bot removed the 🤞 phase/open Post is being triaged manually label Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💪 phase/solved Post is done
Development

Successfully merging a pull request may close this issue.

3 participants