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.

Sign up for GitHub

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

Jump to bottom

Revisit duplication of ignoreOverrides and overrideId between inputs and options #99123

Closed
bpasero opened this issue Jun 2, 2020 · 2 comments
Closed

Revisit duplication of ignoreOverrides and overrideId between inputs and options #99123

bpasero opened this issue Jun 2, 2020 · 2 comments
Assignees
@eamodio @bpasero @mjbvz
Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders under-discussion Issue is under discussion for relevance, priority, approach workbench-editors Managing of editor widgets in workbench window
Milestone
June 2020

Comments

@bpasero
Copy link
Member

bpasero commented Jun 2, 2020

I see we have IBaseResourceEditorInput.overrideId and now also IEditorOptions.overrideId and I think this property should not be on the options of an editor.

At the same time I see we have ignoreOverrides and wonder if the two concepts could be merged, which I think Matt introduced.

Personally I feel this belongs on the editor and not options. Maybe overrideId could have a special value to signal that the override should be ignored.
@bpasero bpasero added debt Code quality issues under-discussion Issue is under discussion for relevance, priority, approach workbench-editors Managing of editor widgets in workbench window labels Jun 2, 2020
@bpasero bpasero added this to the June 2020 milestone Jun 2, 2020
@bpasero
Copy link
Member Author

bpasero commented Jun 2, 2020

If we have more places where override-identifiers are part of any interface or API, please speak up.

bpasero added a commit that referenced this issue Jun 6, 2020
@bpasero
editors - remove overrideId from input (#99123)
c15e3a6
bpasero added a commit that referenced this issue Jun 6, 2020
@bpasero
editors - merge ignoreOverrides and overrideId into one (#99123)
0350010
@bpasero bpasero closed this as completed in 20107ba Jun 8, 2020
@bpasero
Copy link
Member Author

bpasero commented Jun 8, 2020

@eamodio @mjbvz take note of 20107ba, there is now only override as editor option with:

  • undefined: let editor decide
  • false: disable overrides (previously ignoreOverride)
  • string: a specific editor override to use

@github-actions github-actions bot locked and limited conversation to collaborators Jul 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@eamodio eamodio

@bpasero bpasero

@mjbvz mjbvz

Labels
debt Code quality issues insiders-released Patch has been released in VS Code Insiders under-discussion Issue is under discussion for relevance, priority, approach workbench-editors Managing of editor widgets in workbench window
Projects
None yet
Milestone
June 2020
Development

No branches or pull requests
4 participants
@eamodio @bpasero @mjbvz and others