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

Improve preference renderer linking #14311

Merged
merged 3 commits into from
Oct 30, 2024
Merged

Conversation

msujew
Copy link
Member

@msujew msujew commented Oct 14, 2024

What it does

Fixes a few minor issues which should massively reduce the amount of Linked preference "<id>" not found. warnings printed to the console:

  1. Don't use a link to window.titleBarStyle when not in Electron mode (the linked preference does only exist in Electron).
  2. Use editor.cursorSurroundingLines instead of cursorSurroundingLines as a link.
  3. Some extensions sometimes link commands in the preferences. We now also use the CommandRegistry to figure out the name of a linked command.
  4. Caches the IDs of the tree nodes of every preference so that we can later find the tree ID again for any preference ID. That way, we don't have to recompute the ID to figure out the real name of a preference (and potentially do that incorrectly).

How to test

  1. Open the application.
  2. Open the settings/preference view
  3. Assert that there are much fewer warnings in the console.
  4. Assert that all links to other preferences are correctly rendered and actually make you jump to that preference.

Review checklist

Reminder for reviewers

@msujew msujew added the preferences issues related to preferences label Oct 14, 2024
@msujew msujew requested a review from tsmaeder October 14, 2024 13:49
@tsmaeder
Copy link
Contributor

@msujew where can I find "links to other preferences"?

@msujew
Copy link
Member Author

msujew commented Oct 29, 2024

@tsmaeder You can link a preference by wrapping it in `#<preferenceId>#`. It's usually used for the description property, see here for an example:

'window.titleSeparator': {
type: 'string',
default: ' - ',
scope: 'application',
markdownDescription: nls.localizeByDefault('Separator used by {0}.', '`#window.title#`')
},

@tsmaeder
Copy link
Contributor

Do you happen to know an example for a command link?

@msujew
Copy link
Member Author

msujew commented Oct 29, 2024

@tsmaeder I only noticed this by accident after I've included the Latex Workshop extension. See this preference that links to #latex-workshop.build# (a command).

@tsmaeder
Copy link
Contributor

When I go to the preference "Latex > Recipe: Default" and click on "LaTeX Workshop: Build LaTeX project.", I get an error in the browser log:

    at DefaultResourceProvider.get (resource.ts:208:31)
    at async MonacoTextModelService.loadModel (monaco-text-model-service.ts:115:26)
    at async ReferenceCollection.getOrCreateValue (reference.ts:187:27)
    at async ReferenceCollection.acquire (reference.ts:173:24)
    at async MonacoEditorProvider.getModel (monaco-editor-provider.ts:93:27)
    at async MonacoEditorProvider.createMonacoEditor (monaco-editor-provider.ts:197:23)
    at async MonacoEditorProvider.doCreateEditor (monaco-editor-provider.ts:123:24)
    at async EditorPreviewWidgetFactory.constructEditor (editor-preview-widget-factory.ts:42:28)
    at async EditorPreviewWidgetFactory.createEditor (editor-widget-factory.ts:52:27)
    at async EditorPreviewWidgetFactory.createWidget (editor-preview-widget-factory.ts:34:24)

Is there another way to exercise this feature?

@msujew
Copy link
Member Author

msujew commented Oct 29, 2024

@tsmaeder I'm confused, it's not supposed to execute anything when you click on it. The feature is just there to actually show the name of the command - otherwise it will just render the original markdown code block.

@tsmaeder
Copy link
Contributor

@msujew and yet you get the exception each time you click on the "link". My suspicion is that it's trying to open a link, but since there is no href, it's ending up opening "file:///". I kind of remember that we listen to folks opening links in the electron window somewhere.

@msujew
Copy link
Member Author

msujew commented Oct 30, 2024

@tsmaeder Ah, yeah that makes sense. I'll render it into a non-a element instead, since it's not an actual link anyway.

@msujew msujew merged commit ea62f67 into master Oct 30, 2024
11 checks passed
@msujew msujew deleted the msujew/fix-preference-linking branch October 30, 2024 13:17
@github-actions github-actions bot added this to the 1.55.0 milestone Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
preferences issues related to preferences
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants