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

New settings editor should work for some compound setting types #55794

Closed
roblourens opened this issue Aug 3, 2018 · 6 comments · Fixed by #56412
Closed

New settings editor should work for some compound setting types #55794

roblourens opened this issue Aug 3, 2018 · 6 comments · Fixed by #56412
Assignees
Labels
feature-request Request for new features or functionality settings-editor VS Code settings editor issues verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@roblourens
Copy link
Member

roblourens commented Aug 3, 2018

  • string | null: Check whether it's ok to map "" to null
  • enum | null: Add 'null' or "None" item to enum
  • number | null: "" => null
  • boolean | null: ?? I don't see this in vscode's core settings

Also, enum | boolean. Example:

"git.autoRepositoryDetection": {
    "type": [
        "boolean",
        "string"
    ],
    "enum": [
        true,
        false,
        "subFolders",
        "openEditors"
    ],
@roblourens roblourens added feature-request Request for new features or functionality settings-editor VS Code settings editor issues labels Aug 3, 2018
@roblourens roblourens added this to the August 2018 milestone Aug 3, 2018
@roblourens roblourens self-assigned this Aug 3, 2018
@roblourens roblourens changed the title New settings editor should work for null | something setting types New settings editor should work for some compound setting types Aug 3, 2018
@roblourens roblourens mentioned this issue Aug 3, 2018
38 tasks
@JacksonKearl
Copy link
Contributor

  // Specifies the icon theme used in the workbench or 'null' to not show any file icons.
  //  - null: No file icons
  //  - vs-minimal
  //  - vs-seti
  "workbench.iconTheme": "vs-seti",

  // List of tags, comma separated, where the content shouldn't be reformatted. 'null' defaults to the 'pre' tag.
  "html.format.contentUnformatted": "pre,code,textarea",

  // List of tags, comma separated, that should have an extra newline before them. 'null' defaults to "head, body, /html".
  "html.format.extraLiners": "head, body, /html",

  // Maximum number of line breaks to be preserved in one chunk. Use 'null' for unlimited.
  "html.format.maxPreserveNewLines": null,

  // List of tags, comma separated, that shouldn't be reformatted. 'null' defaults to all tags listed at https://www.w3.org/TR/html5/dom.html#phrasing-content.
  "html.format.unformatted": "wbr",

  // Sets the locale used to report JavaScript and TypeScript errors. Requires using TypeScript 2.6.0 or newer in the workspace. Default of `null` uses VS Code's locale.
  "typescript.locale": null,

These are the settings explicitly null | string that I could find with a simple search. Of these, the HTML ones are the only concern. For most of them we should probably just get rid of accepting null, as it doesn't make a ton of sense to have null just mean either default or some secondary default.

But "html.format.unformatted" has very different behavior between null and '' so we'll have to decide on something to do there.

@JacksonKearl
Copy link
Contributor

JacksonKearl commented Aug 14, 2018

@bpasero
Copy link
Member

bpasero commented Aug 29, 2018

@JacksonKearl @roblourens can you add some steps for how to verify?

@bpasero bpasero added the verification-steps-needed Steps to verify are needed for verification label Aug 29, 2018
@roblourens
Copy link
Member Author

roblourens commented Aug 29, 2018

Here are some settings with compound types

  • git.autoRepositoryDetection
  • html.format.maxPreserveNewLines

Test that they work as you'd expect.

I can't remember which extension has the nullable enum but you can test it with a contribution like

"configuration":{
    "title": "testext",
    "properties": {
        "testext.setting1": {
            "type": [
                "null",
                "string"
            ],
            "enum": [
                "option 1",
                "option 2",
                null
            ],
            "description": "test setting"
        }
    }
}

@bpasero bpasero removed the verification-steps-needed Steps to verify are needed for verification label Aug 29, 2018
@bpasero
Copy link
Member

bpasero commented Aug 30, 2018

@roblourens seems to work for the first one, but not second. e.g. here I cannot type "null":

image

I can only type numbers (I am on Windows).

@bpasero bpasero reopened this Aug 30, 2018
@bpasero bpasero modified the milestones: August 2018, September 2018 Aug 30, 2018
@bpasero bpasero added the verification-found Issue verification failed label Aug 30, 2018
@roblourens
Copy link
Member Author

For the null|number type, an empty input box will correspond to null in the json

@roblourens roblourens removed the verification-found Issue verification failed label Aug 30, 2018
@bpasero bpasero modified the milestones: September 2018, August 2018 Aug 30, 2018
@bpasero bpasero added the verified Verification succeeded label Aug 30, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 14, 2018
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 settings-editor VS Code settings editor issues verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants