Skip to content

[#3958] fix key command action handler gets deleted by content assist#3959

Open
ghentschke wants to merge 1 commit intoeclipse-platform:masterfrom
ghentschke:fix-broken-key-handler
Open

[#3958] fix key command action handler gets deleted by content assist#3959
ghentschke wants to merge 1 commit intoeclipse-platform:masterfrom
ghentschke:fix-broken-key-handler

Conversation

@ghentschke
Copy link
Copy Markdown

fixes #3958 by handling multiple calls on assistSessionStarted(ContentAssistEvent)

…y content assist

fixes eclipse-platform#3958 by handling multiple calls on
assistSessionStarted(ContentAssistEvent)
@ghentschke
Copy link
Copy Markdown
Author

@mickaelistria can you please take a look at this?

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 3, 2026

Test Results

   852 files  ±0     852 suites  ±0   52m 59s ⏱️ - 4m 59s
 7 940 tests ±0   7 697 ✅ ±0  243 💤 ±0  0 ❌ ±0 
20 322 runs  ±0  19 667 ✅ ±0  655 💤 ±0  0 ❌ ±0 

Results for commit 46f8d62. ± Comparison against base commit 4888468.

@mickaelistria
Copy link
Copy Markdown
Contributor

I think this patch is good and is the simplest way to address the reported issue.
Sure, one could argue that this method should not be called multiple times, but anyway, there is no real way to prevent a method from being called multiple times, so making it multiple-calls-able seems more robust that working on the caller to call it only once.

Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks fine to me as well. Evidently assistSessionEnded is already guarded for being called multiple times or for being called without assistSessionStarted having been called, so guarding multiple calls to assistSessionEnded without a subsequent call to assistSessionEnded seems safe enough.

@ghentschke
Copy link
Copy Markdown
Author

ghentschke commented May 4, 2026

Thank for your fast replies @mickaelistria and @merks
IMO it would be great if we can merge this soon, as it the editor behavior is quite annoying for users.
Can it be part of 2026-06 or is it too late?

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.

Key command action handler gets deleted by content assist

3 participants