Vscode-spell-checker: Support VSCode Multi Root Workspace

[Jason3S]

Extension-Authoring:-Adopting-Multi-Root-Workspace-APIs

Multi Root is a breaking change for this extension.

[seanmcbreen]

Hi,

I'm from the VS Code team (we have chatted before 😄) and I'm reaching out with some additional resources for adopting multi-root workspaces.

But first let me again say thank you for building Code Spell Checker which is one of the most popular extensions for VS Code - and I've heard nothing but good things post the transition from my old sample checker. We really appreciate the work you have done to make VS Code better.

The purpose of this issue is that we are very close to releasing a significant new feature for VS Code - Multi-root Workspaces - which is the most requested missing feature. If you have not heard of multi-root workspaces we have described this in several of our recent updates. Today it's only enabled in our insiders release but our intent is to enable it for all users in our next update, targeted for release in the first week of November.

This is a pretty significant change and over the last four months we have updated pretty much all of VS Code to be Multi-root aware as well as updated all of our own extensions. In the process of doing this we have created a set of guidelines for Extension Authors to follow to ensure their extension can fully leverage the awesomeness that is Multi-root workspaces.

Our Request:

Having looked at your extension we think changes are required in the following areas:

• Your extension makes use of the (now deprecated) workspace.rootPath property, there is a proposed migration path and sample for this case.
• Your extension contributes or changes settings you need to review whether some of the settings can be applied on a resource (= file location) level instead of being global (= workspace level).

How Can We Help/Resources:

We realize we are asking for your help here - our goal is to make sure that your extensions works well with multi-root and continues to be loved by the community. The guide above outlines the changes that are required and we have a samples as well.

But we are sure you will have questions and we thought one way we could look to help is by getting a number of the team members (myself included) to be ready to respond to questions on a Slack Channel that we will dedicate to this issue. To join that channel simply follow this link.

Again thank you for being part of the VS Code community - we appreciate everything you have done. With the release of Multi-root Workspaces we will be shipping our most requested feature, together we can really help this feature light up for users.

Happy Coding!
Sean McBreen

// cc @bpasero

[Jason3S]

Hello Sean,

Thank you for the support and recognition. I plan on taking a look at it this weekend. I might end up filing a few bugs about things I have already noticed with respect to reading and writing to the workspace configuration (i.e. workspace.getConfiguration().inspect() returns stale values).

Due to the way the spell checker configuration settings are propagated, I am going to need to start a language server per root or create a concept of workspace context with in the server. I'll know more after I take a deeper look.

Again, thank you for following up.

Kind Regards,
Jason

[bpasero]

@Jason3S great to hear that. If you could try to come up with reproducible steps for the bugs you see, please file the issues to our repo.

[seanmcbreen]

Request: if you have prepared your extension for Multi-root Workspaces we would love you to add multi-root ready to your package.json i.e. "keywords": ["multi-root ready"]. If you upgrade to VSCE 1.32.0 this can be added to any existing keywords and will not be counted against the maximum of 5 allowed keywords.

This will help us keep track of adoptions and also help users know which extensions have been fully tested. Remember we are here to help should you need it on the Slack channel.

Happy Coding
Sean McBreen

[Jason3S]

@seanmcbreen,

I am still working on this. It has taken me longer than I expected.

I'll add the keywords when it is ready.

Kind Regards,
Jason
PS. In Redmond this week.

[seanmcbreen]

Jason I'd say come and say hi - but I've unfortunately been sick at home all week - do look us up one time if you come to redmond often smcbreen @ msft

[seanmcbreen]

27 and we should be in next week for sure - happy to grab a coffee.

[Jason3S]

@seanmcbreen, I am still working on this. It became more complicated than expected.

Some of the warning messages are very hard to hunt down:

[streetsidesoftware.code-spell-checker] Accessing a window scoped configuration for a resource is not expected. To associate '[makefile]' to a resource, define its scope to 'resource' in configuration contributions in 'package.json'.
extensionHostProcess.js:4
[streetsidesoftware.code-spell-checker] Accessing a window scoped configuration for a resource is not expected. To associate '[markdown]' to a resource, define its scope to 'resource' in configuration contributions in 'package.json'.
extensionHostProcess.js:4
[streetsidesoftware.code-spell-checker] Accessing a window scoped configuration for a resource is not expected. To associate '[yaml]' to a resource, define its scope to 'resource' in configuration contributions in 'package.json'.

The challenge is to know if it is the client or server accessing the resource and which parent resource. The other challenge is being required to read the settings twice.

Because there are a lot of possible settings I tend to read just the root setting cspell. Now I have to make two reads and add special code to figure out which settings should come from which version (workspace or folder).

I guess I was expecting a different behavior. When reading the setting, I was hoping all I needed to do was supply the url of the resource. My expectation was that the folder level and workspace level would be merged for reading (similar to how the global settings are merged in).

I would have expected an error/warning if I tried to read a folder level setting, but hadn't supplied a resource url. But the other way around was unexpected.

For writing to a setting, I would expect to be required to supply the correct target.

This wasn't intended as a complaint, but more of some feedback on why it has been taking longer than I expected to integrate the multi-root workspace.

[bpasero]

@sandy081 can you help @Jason3S out

[sandy081]

@Jason3S From the warnings, it looks like you are asking for settings which are not defined as Resource scoped using a resource. You can take any of the following actions

• If they are not resource scoped settings, you do not need to pass in a resource while accessing the setting.

• If they are resource scoped settings, you have to define them as Resource scoped while registering in the package.json

Please take a look at Settings wiki for more help.

[Jason3S]

@sandy081,

Thank you. That is my understanding as well.

I guess my question is: Should I care about the warning?

I guess my usage case is not the normal usage case. I take the entire settings object and send it to a library.

I read all the settings in at once and merge them with existing and default settings. The issue is that I read the top level section, not individual settings. In the merge process it loops through all the fields in the object returned by workspace.getConfiguration(undefined, resource).get('cSpell'). Since the window resources are returned with resource requests (and vice versa), I would need to add filters to remove resource level settings before merging them with window level settings.

Basically, I would need to make two calls to getConfiguration one for resource and one for window, filter each objected based upon how it was requested and merge the results.

The other challenge, is that some settings can make sense at both levels: The user would want the workspace setting unless overridden by the resource level setting.

[sandy081]

If you are sure of what you are doing, it makes sense to ignore that warning.