Skip to content

Document fontLigatures format#4094

Closed
lf- wants to merge 1 commit intomicrosoft:masterfrom
lf-:patch-2
Closed

Document fontLigatures format#4094
lf- wants to merge 1 commit intomicrosoft:masterfrom
lf-:patch-2

Conversation

@lf-
Copy link

@lf- lf- commented Oct 31, 2020

I had to read the source code to figure out how this property worked. Let me know if there's anywhere else I should improve the documentation of this setting.

@gregvanl
Copy link

gregvanl commented Nov 3, 2020

HI @lf- Thank you for tracking this down and submitting a PR. The settings section is actually auto-generated from the in-product defaultSettings.json (Preferences: Open Default Settings (JSON)) so the description fix should be made in the vscode product repo in https://github.com/microsoft/vscode/blob/master/src/vs/editor/common/config/editorOptions.ts.
Search for the class EditorFontLigatures and you can find the description to update.
I think you'll have to format the description as a single string, rather than multiple lines.

@gregvanl gregvanl closed this Nov 3, 2020
@lf-
Copy link
Author

lf- commented Nov 3, 2020

@gregvanl In that case, I'd say that the current documentation generator is buggy! Although the option should mention in its own description that there are multiple types possible and I should PR that, the documentation generator should emit the docs for both of the cases in the anyOf as defined here: https://github.com/microsoft/vscode/blob/master/src/vs/editor/common/config/editorOptions.ts#L1410

@gregvanl
Copy link

gregvanl commented Nov 3, 2020

Hi @lf- Agreed, that sometimes the generator does the right thing, for example with enumerated values (see editor.unusualLineTerminators), but not in the case of anyOf.
The quick fix would be to update the description string but one could also look into adding better support for anyOf in the defaultSettings.json output.

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.

2 participants