microsoft / vscode

Visual Studio Code
https://code.visualstudio.com
MIT License
162.73k stars 28.72k forks source link

Add Window Setting Scope that does not inherit user (local) settings on remote machines #159209

Open browntarik opened 2 years ago

browntarik commented 2 years ago

Our extension would like to introduce a setting that requires a window scope but also does not allow remote sessions to inherit user (local) settings. The current setting will allow users to override file path case sensitivity on Windows machines however we do not want to inherit this behavior if the user has opened a workspace on a remote machine as this could be breaking behavior. As a workaround, we are currently setting the scope of the setting to "machine-overridable" and just ignoring any setting changes at the folder level. Ideally, we could just have this setting as window scope where the user cannot change the setting at the folder level and the setting isn't inherited on remote machines.

sandy081 commented 2 years ago

Have you tried application scope?

browntarik commented 2 years ago

No, mainly because we do not want settings to be overwritten on a workspace folder level if there is a multi-root scenario which has more than one folder added to the workspace. The setting should only be set per workspace. All folders in a workspace should have the same file path case sensitivity. Maybe a better example is machine-overridable, if machine overridable did not let the setting be overwritten on a workspace folder level, it would be a perfect scope for us because it also does not inherit user settings remotely

browntarik commented 2 years ago

image

browntarik commented 2 years ago

Here is a matrix of the logic we have found and why application, window and machine overridable scope does not cover our necessary case. If we are incorrect please let us know

sandy081 commented 2 years ago

So this setting shall be mutually exclusive for machine. If defined in local take it, If defined in remote take it. If defined in both, local wins?

browntarik commented 2 years ago

Correct, because we do not want to inherit user settings on a remote machine as these could conflict potentially.

Colengms commented 2 years ago

I think the issue here is that there are various scopes that do not have "don't inherit remotely" alternatives.

Consider a setting that involves a file system path. You may want to be able to override that setting at the workspace level and/or the workspace folder level. And a setting containing a (local) file path should never be inherited remotely.

isidorn commented 2 years ago

@browntarik @Colengms thanks for opening this issue. Can you please explain your use case better. What settings are these? How exactly are you impacted as an extension? We would like to understand this better. Thanks

Colengms commented 2 years ago

Hi @isidorn . In this case, the setting was a bool to override the default case sensitivity/insensitivity of our use of the file system, which we are technically unable to vary at the workspace folder level (in multi-root), but can (and should) vary at the workspace level (implicitly the workspace folder level, if not using multi-root). This setting was motivated by Windows users who open a folder on a case sensitive file share. As the setting relates to the file system, it's undesirable to inherit that setting in a remote (if they set it at the user level), as a remote would almost assuredly involve a completely different file system.

If we were technically able to support this on a workspace-folder level, we might have wanted to. The issue here is that there are no available scopes that would allow us to vary a setting at the workspace and not also inherit a user level value in a remote. For any file path or file system related setting that should be allowed to be varied at the workspace and/or workspace-folder level, it would almost assuredly be undesirable to automatically inherit in a remote.

Below is an updated copy of a table that helps us understand how the scopes work (they're confusing). Let us know if this is inaccurate.

My suggestion would be to complete the matrix in the table below, so all combinations that make sense have an available scope. (Ignoring language-overridable, and ignoring combinations with contradictory implications.) That would seem to leave the following combinations:

no, no, yes, yes (suggested by #131126 yes, no, no, yes (suggested by this issue)

<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns="http://www.w3.org/TR/REC-html40">

 

 

Scope | VS Code description | Overridable in workspace (or workspace folder in a single root scenario) | Overridable in workspace folder (in a multiroot scenario which has one or more folders) | Remote will inherit local user settings | Overridable remotely | Overridable at language level -- | -- | -- | -- | -- | -- | -- application | Settings that apply to all instances of VS Code and can only be configured in user settings. | no | no | yes | no | no window | Windows (instance) specific settings which can be configured in user, workspace, or remote settings. | yes | no | yes | yes | no resource | Resource settings, which apply to files and folders, and can be configured in all settings levels, even folder settings. | yes | yes | yes | yes | no machine | Machine specific settings that can be set only in user settings or only in remote settings. For example, an installation path which shouldn't be shared across machines. | no | no | no | yes (implicitly, as it's not inherited) | no machine-overridable | Machine specific settings that can be overridden by workspace or folder settings. | yes | yes | no | yes (implicitly, as it's not inherited) | no language-overridable | Resource settings that can be overridable at a language level. | yes | yes | yes | yes | Yes https://github.com/microsoft/vscode/issues/131126 Application, but can be overriden remotely | (Needed) | no | no | yes | yes | no https://github.com/microsoft/vscode/issues/159209 Window, but does not inherit remotely | (Needed) i.e. For caseSensitiveFileHandling | yes | no | no | yes (implicitly, as it's not inherited) | No

 

 

VSCodeTriageBot commented 1 year ago

Hey @rzhao271 @sandy081, this issue might need further attention.

@browntarik, you can help us out by closing this issue if the problem no longer exists, or adding more information.

sandy081 commented 1 year ago

Just stepping back and understanding the requirement and from the description

Our extension would like to introduce a setting that requires a window scope but also does not allow remote sessions to inherit user (local) settings.

Does the machine scope is the one you want to use here. Since FS is machine specific, it goes well with that. For eg., if FS is case sensitive locally then user can set it to true in local user settings, and not in remote, then can set it to false in remote machine settings. Is not it enough or am I missing more here?

Colengms commented 1 year ago

@sandy081 , as noted in the table:

<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:dt="uuid:C2F41010-65B3-11d1-A29F-00AA00C14882" xmlns="http://www.w3.org/TR/REC-html40">

 

 

Scope | VS Code description | Overridable in workspace (or workspace folder in a single root scenario) | Overridable in workspace folder (in a multiroot scenario which has one or more folders) | Remote will inherit local user settings | Overridable remotely | Overridable at language level -- | -- | -- | -- | -- | -- | -- machine | Machine specific settings that can be set only in user settings or only in remote settings. For example, an installation path which shouldn't be shared across machines. | no | no | no | yes (implicitly, as it's not inherited) | no

 

 

Note the "Overridable in workspace (or workspace folder in a single root scenario)" column. If we only wanted to allow the value to be varied at the scope of an entire machine, the machine scope would work for us. But, for this specific setting, we require the setting to be variable per 'workspace' (the first "no" column needs to be 'yes'), but CANNOT vary it per 'workspace folder' for technical reasons.

Having it vary per workspace is a primary requirement, but the nature of the setting (a path) requires that it should not be automatically applied to other machines...

In my previous response, I provided an analysis of how these scopes break out into a matrix, and how the 2 missing scopes from this matrix are indeed the ones which we've had to open issues against due to logically arriving upon their necessity and discovering them missing.

sandy081 commented 1 year ago

@Colengms Thanks for the explanation and pointing out the missing scopes.

But, for this specific setting, we require the setting to be variable per 'workspace' (the first "no" column needs to be 'yes')

Given the file case-senstivity example, I expect it is a machine setting rather than only workspace setting. If one wants to override at workspace level, then you can scope it to machine-overridable right?

but CANNOT vary it per 'workspace folder' for technical reasons.

Can you please elaborate on this?

Colengms commented 1 year ago

Hi @sandy081 . If we could vary the setting at a workspace folder level, then yes, we could use machine-overridable, but we cannot.

By "technical reasons", I mean that our native LSP process is not currently architected in such a way that can we vary the setting on a per workspace-folder basis, as only one instance of the process is running per workspace and the setting must be applied across that instance.

sandy081 commented 1 year ago

Got it. IMO file case-sensitive setting is machine overridable setting and it can vary per workspace folder because theoretically, each workspace folder can be from different source. Since, you have a technical limitation, you can read this settings without passing workspace folder. For eg:

const value = vscode.workspace.getConfiguration().get('${setting}');

Returned value is

I assume this is what you are looking for?

Colengms commented 1 year ago

Hi @sandy081 . Until/unless this issue is address, yes, that is one of our options. That incorrectly allows the user to vary the setting on a per workspace-folder level, without obvious feedback to them that it will be ignored for some workspace folders in a multi-root workspace. Our other option is window scope, which correctly cannot be varied at a per workspace-folder level, but must unfortunately be explicitly set remotely in order to avoid inheriting the value, if inheriting is not desirable.

I think we've made a pretty clear case here that these scopes correspond with a matrix of behaviors, and that there are 2 useful (required) combinations that are missing from the matrix. Also, the way these scopes are documented is vague and confusing. We've repeatedly had to correct our use of scopes due to misunderstandings (which we created that table to help with). I think VS Code would benefit from clearly indicating this matrix in the description of the scopes, and addressing the missing valid combinations.

sandy081 commented 1 year ago

@Colengms I really appreciate for coming up with the scopes matrix, that not only helps you and us but also the entire community. I have created a doc issue to include this matrix in our doc - https://github.com/microsoft/vscode-docs/issues/5871

According to the use case you described the scope you wanted is machine-overridable scope that is existing. Due to the technical limitations on your side, you want this to behave like machine-window-overridable scope which is hard to name and explain as a concept. Based on your use case (file-case-sensitivity), making it machine-window-overridable is confusing/wrong if user has workspace folders with different case sensitivity. Marking this as machine-overridable you can atleast check this setting at each folder and if they are varying, then you can inform user that you do not support such config and let user take proper action before proceeding.

I would be happy to complete/add more scopes to the matrix if there is a compelling purpose or use case.