Skip to content

Fix #205836, multiple menubar instances register same command twice.#205848

Closed
yiliang114 wants to merge 20 commits intomicrosoft:mainfrom
yiliang114:puny-whitefish
Closed

Fix #205836, multiple menubar instances register same command twice.#205848
yiliang114 wants to merge 20 commits intomicrosoft:mainfrom
yiliang114:puny-whitefish

Conversation

@yiliang114
Copy link
Contributor

@yiliang114 yiliang114 commented Feb 21, 2024 •
edited
Loading

Fix #205836

bug33

@bpasero
Copy link
Member

bpasero commented Feb 21, 2024

✋ I think we cannot really change command identifiers here.

alexr00 and others added 11 commits February 21, 2024 20:17
@alexr00 @yiliang114
Aggressive caching with Open Current File as Release Notes (microsoft…
d117a0d 
…#205846)

Fixes microsoft#205675
@hediet @yiliang114
Improves softAssert function
6d0764f
@bpasero @yiliang114
Something should happen when the flashing microphone is clicked (fix m…
1a7181f 
…icrosoft#205670) (microsoft#205842)

* Something should happen when the flashing microphone is clicked (fix microsoft#205670)

* switch to action bar

* .
@jrieken @yiliang114
make sure LM provider error bubble all the way, reject response strea…
0f4eacc 
…ms and result promise (microsoft#205849)

fixes microsoft#205722
@lszomoru @yiliang114
Git - fix reopen closed repositories action visibility issue (microso…
ad0879a 
…ft#205851)
@alexr00 @yiliang114
Fix issue with settings not being respected in release notes (microso…
c16d8cb 
…ft#205852)

Settings with values separated are not respected in release notes
Fixes microsoft#205678
Also fixes microsoft#205742
@alexr00 @yiliang114
"Try Feature" tooltip sounds awkward (microsoft#205841)
7bf0d5b 
* "Try Feature" tooltip sounds awkward
Fixes microsoft#205741

* Fix test
@bpasero @yiliang114
actionbar - animated is an obsolete property (microsoft#205854)
2db24cc
@bpasero @yiliang114
editors - use canSerialize properly (microsoft#205856)
8245a85
@hediet @yiliang114
Sets focus border for focused editors
1404b83
@yiliang114
Merge remote-tracking branch 'origin/main' into puny-whitefish
7d37a87
@yiliang114
Copy link
Contributor Author

yiliang114 commented Feb 21, 2024

✋ I think we cannot really change command identifiers here.✋我认为我们不能在这里真正改变命令标识符。

Yes, it may indeed not be a good decision. Let me think again

@yiliang114
Copy link
Contributor Author

yiliang114 commented Feb 22, 2024

✋ I think we cannot really change command identifiers here.✋我认为我们不能在这里真正改变命令标识符。

@bpasero I thought of a new solution, because CustomMenubarControl is used in only two places and the instances on both sides are mutually exclusive (the old instance needs to be destroyed before the new one can continue to be initialized), then we can store a static property on the class whose value is the previous menuBar instance.

@yiliang114
Copy link
Contributor Author

yiliang114 commented Feb 27, 2024

✋ I think we cannot really change command identifiers here.✋我认为我们不能在这里真正改变命令标识符。

@bpasero I thought of a new solution, because CustomMenubarControl is used in only two places and the instances on both sides are mutually exclusive (the old instance needs to be destroyed before the new one can continue to be initialized), then we can store a static property on the class whose value is the previous menuBar instance.我想到了一个新的解决方案，因为 CustomMenubarControl 仅在两个地方使用，并且两侧的实例是互斥的 (需要先销毁旧实例，然后才能继续初始化新实例)，然后我们可以在类上存储一个静态属性，其值是以前的menuBar实例。

hi @bpasero Can you take time to help me see this pr?

@yiliang114 yiliang114 closed this Dec 16, 2025
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Jan 30, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

@sbatten sbatten

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Switching menubar display position failed with error

7 participants

@yiliang114 @bpasero @sbatten @alexr00 @hediet @jrieken @lszomoru