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

Multi root (vscode next) #252

Merged
merged 5 commits into from
Nov 21, 2017
Merged

Multi root (vscode next) #252

merged 5 commits into from
Nov 21, 2017

Conversation

CiGit
Copy link
Member

@CiGit CiGit commented Nov 3, 2017

Adapt settings for resource usage. ie settings which are relevant to a file.

remove rootpath usage.

Will need to make test on an insider version. (Welcome testers !)

fix #243

Edit: Made some tests on insider with 2 folders. Was working 😄

CiGit added 2 commits November 3, 2017 15:25
vscode dep bump
Simply remove them
It removes 1 "feature", a warning message which is in the output panel anyway.
Where it does count, multi-root is already taken into account.
@felixfbecker
Copy link
Contributor

Nice! Is there a way to enable prettier only per folder? I.e. a folder setting "prettier.enable": true/false. That would be especially important for multi-folder workspaces to not cause unwanted formatting changes

package.json Outdated
@@ -19,7 +19,7 @@
"url": "https://github.com/prettier/prettier-vscode/issues"
},
"engines": {
"vscode": "^1.15.0"
"vscode": "^1.17.0"
},
"categories": [
"Formatters"
Copy link
Member

@azz azz Nov 4, 2017

Choose a reason for hiding this comment

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

Going to add multi-root ready?

#243 (comment)

Copy link
Member Author

Choose a reason for hiding this comment

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

eventually yes.

Just wanted to really test it first.

@CiGit
Copy link
Member Author

CiGit commented Nov 4, 2017

@felixfbecker This is not possible. Either you register a formatter for the entire instance or you don't.
However .prettierignore may do what you want.

@felixfbecker
Copy link
Contributor

@CiGit

Either you register a formatter for the entire instance or you don't

Sorry I don't understand. Why is this not possible (but .prettierignore is)?

@CiGit
Copy link
Member Author

CiGit commented Nov 4, 2017

I may have misunderstood you question. To me you wanted to be able to use any other formatter (at least the default one) on other folder. ie enable the extension per folder. This may be a feature from vscode.

If it's just a toggle, on/off, this may be possible, but I would be against it. #199 would already do it. Once implemented.

@azz
Copy link
Member

azz commented Nov 4, 2017

If you're looking to turn off formatOnSave for a given workspace folder, I believe you can just put "formatOnSave": false in .vscode/settings.json in that folder and it will work.

@felixfbecker
Copy link
Contributor

I think #199 would solve it. "Format document" needs to use a different formatter depending on the workspace folder.

@rothfels
Copy link

rothfels commented Nov 4, 2017

@felixfbecker afaict a formatter extension must register itself (and therefore override default formatters shipped w/ vscode) before we have specific information about the workspaces in a multi-root setting. The .prettierignore option works to disable prettier formatting on a workspace root, but the prettier extension will have still been registered and overridden the default formatter (even though it's a noop on ignored files). I don't think there's currently a way to dynamically register a formatter extension based on properties/config of the workspace root. This is unfortunate because (especially in a multi-root context) I imagine many people will run into an issue lacking the ability to run multiple formatters for the same language id simultaneously.

microsoft/vscode#15611 might be the appropriate solution

@CiGit CiGit changed the title [WIP] Multi root (vscode next) Multi root (vscode next) Nov 8, 2017
@CiGit
Copy link
Member Author

CiGit commented Nov 8, 2017

Ready to review! I've nothing more to test or add

Copy link
Contributor

@RobinMalfait RobinMalfait left a comment

Choose a reason for hiding this comment

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

Code looks good to me! Couldn't test functionally because I forgot my charger...

@CiGit CiGit merged commit 8f31fa2 into prettier:master Nov 21, 2017
@CiGit CiGit deleted the multi-root branch November 21, 2017 21:09
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.

Multi-root Workspaces and Prettier
5 participants