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

[Settings GUI] String array: No help in filling enum values #77458

Closed
usernamehw opened this issue Jul 16, 2019 · 9 comments
Closed

[Settings GUI] String array: No help in filling enum values #77458

usernamehw opened this issue Jul 16, 2019 · 9 comments
Assignees
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan settings-editor VS Code settings editor issues
Milestone

Comments

@usernamehw
Copy link
Contributor

#77427

"type": "array",
"items": {
	"type": "string",
	"enum": [
		"error",
		"warning",
		"info",
		"hint"
	]
},

settings.json:

Screenshot (29)

settings GUI:

Screenshot (30)

@octref octref added feature-request Request for new features or functionality settings-editor VS Code settings editor issues labels Jul 30, 2019
@octref octref assigned roblourens and unassigned octref Jul 30, 2019
@octref octref added this to the Backlog milestone Jul 30, 2019
@eamodio
Copy link
Contributor

eamodio commented Aug 9, 2019

I think this is FAR worse than just not providing help when using enums, because as far as I can tell it doesn't even validate values against the allowed enum values. Which is basically inviting users to enter invalid/bad data which can cause extensions to not function properly.

While this was always possible before, the user would not only get intellisense on the allowed values you'd also get error warning about invalid values.

Can this be something that is prioritized for at least the next iteration (though IMO this should warrant a patch release when fixed)?

/cc @roblourens @kieferrm

@usernamehw
Copy link
Contributor Author

Validation issue was closed as-designed: #77456

@roblourens
Copy link
Member

@octref and I talked about this, I thought there was an open issue for validation somewhere.

@KamasamaK
Copy link

Additional validation wouldn't be as necessary if it just worked like standalone enums do with a drop-down instead of a text input.

@roblourens
Copy link
Member

Validation is still necessary because you can type whatever you want or the validation criteria can change.

@octref octref modified the milestones: Backlog, August 2019 Aug 9, 2019
@octref octref modified the milestones: August 2019, Backlog Aug 22, 2019
@octref
Copy link
Contributor

octref commented Aug 22, 2019

Errors are added as part of #77459 (besides enum, pattern, minItems, maxItems are also supported).

image

So I'm putting this one into backlog.

@rzhao271
Copy link
Contributor

Just noting down some findings so far.

There are settings that are lists of enums, such as git.checkoutType, or jupyter.widgetScriptSources. git.checkoutType doesn't allow for duplicate elements ("uniqueItems": true), and the order of the elements matters. For example, if "remote" comes before "local" in the array, and a user runs the "Checkout to..." command, then the remote URLs show up first.
Therefore, one way to display a list of enum values would be as a list of dropdowns.

There are also settings of type object, where the keys and/or values are enums. These cases are actually rendered properly by the settings editor (https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts#L908). Yet, the settings editor is still unable to currently render objects with primitive non-string values (i.e. booleans/numbers/integers) right now.

@rzhao271 rzhao271 modified the milestones: Backlog, May 2021 May 24, 2021
@rzhao271
Copy link
Contributor

rzhao271 commented May 25, 2021

I was looking more into the ListSettingWidget, and got confused by when item.sibling would be non-null in the following case: https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/preferences/browser/settingsWidgets.ts#L529

I noticed that the exclude widget set sibling here https://github.com/microsoft/vscode/blob/main/src/vs/workbench/contrib/preferences/browser/settingsTree.ts#L75, but I thought that even that code path wasn't being used, because the exclude widget didn't have the option to add a when clause. It turns out that one has to set the when clause in the json file first, and then they can view and edit it in the settings editor.

@rzhao271
Copy link
Contributor

\closedWith ff39a97

An enhancement would be adding a mechanism to help rearrange items of the array while in view mode.

@rzhao271 rzhao271 added the on-release-notes Issue/pull request mentioned in release notes label Jul 2, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Jul 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-request Request for new features or functionality insiders-released Patch has been released in VS Code Insiders on-release-notes Issue/pull request mentioned in release notes on-testplan settings-editor VS Code settings editor issues
Projects
None yet
Development

No branches or pull requests

7 participants
@roblourens @eamodio @octref @rzhao271 @usernamehw @KamasamaK and others