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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Context key change event is triggered when setting the same URI but different instance #63656

Closed
sandy081 opened this issue Nov 22, 2018 · 5 comments
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Milestone

Comments

@sandy081
Copy link
Member

  • Use Resource Context that takes URI as the value
  • Create an URI from a string and set it
  • Context key change event is triggered
  • Create another URI from the same string as in step 2 and set it
    馃悰 Context key change event is triggered again

You can see an infinite loop when running from sources by doing the following change
Change the return to

https://github.com/Microsoft/vscode/blob/1081caf17a7862bf6cef97f2d99b77b19b4d6734/src/vs/workbench/services/preferences/common/preferencesEditorInput.ts#L115

return URI.from({
		scheme: 'vscode-settings',
		path: `settingseditor`
	});

Put a console log before following line

https://github.com/Microsoft/vscode/blob/1081caf17a7862bf6cef97f2d99b77b19b4d6734/src/vs/workbench/browser/parts/editor/titleControl.ts#L212

  • Now Open Settings Editor
  • You will see the indefinite loop.
@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

Yeah.... it's a new instance so the !==-check won't help...

@jrieken jrieken added this to the November 2018 milestone Nov 22, 2018
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Nov 22, 2018
@jrieken
Copy link
Member

jrieken commented Nov 22, 2018

I have pushed a change that implements a proper equality check but this line is still fishy: https://github.com/Microsoft/vscode/blob/8b802549b03e6eee32c4b55843e3183f4d0c7398/src/vs/workbench/browser/parts/editor/titleControl.ts#L211-L212

This is updating the context when being asked for actions, which might very likely cause a call to getEditorAction in return. I have initially added that code but that was before grid and before having a well-defined world in which the title was scoped the an editor. @bpasero I assume things a better today that there is something the titleControl can implement so that it is called whenever its editor changed, right?

@jrieken jrieken closed this as completed Nov 22, 2018
@bpasero
Copy link
Member

bpasero commented Nov 23, 2018

@jrieken possible, I will check in debt week.

@jrieken
Copy link
Member

jrieken commented Nov 23, 2018

Great, we should then also try to keep the menu alive instead of recreating it all the time. I will will then fire events to signal that the UI needs updating.

@sandy081 sandy081 added the verified Verification succeeded label Dec 5, 2018
@vscodebot vscodebot bot locked and limited conversation to collaborators Jan 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants