making sure the then on triggerPaste is not evaluated twice#284961
making sure the then on triggerPaste is not evaluated twice#284961
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes an issue where the then handler on triggerPaste is evaluated multiple times, causing duplicate paste operations. The fix introduces a lock mechanism to prevent the handler from executing more than once.
- Adds a new
electronBugWorkaroundPasteEventLockflag toPasteOptions - Implements a check-and-set lock pattern to guard against duplicate handler invocation
- Adds explanatory comments describing the issue
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/vs/editor/browser/controller/editContext/clipboardUtils.ts |
Adds the electronBugWorkaroundPasteEventLock boolean field to the PasteOptions object |
src/vs/editor/contrib/clipboard/browser/clipboard.ts |
Implements the lock pattern by initializing the lock to false, checking it before executing the paste handler, and setting it to true after the first execution to prevent duplicate runs |
| const triggerPaste = clipboardService.triggerPaste(getActiveWindow().vscodeWindowId); | ||
| if (triggerPaste) { | ||
| logService.trace('registerExecCommandImpl (triggerPaste defined)'); | ||
| PasteOptions.electronBugWorkaroundPasteEventLock = false; |
There was a problem hiding this comment.
The lock initialization at line 324 is confusing because it happens outside the promise chain, making the code flow harder to follow. For consistency with the existing pattern used for electronBugWorkaroundPasteEventHasFired (line 320), consider moving the lock initialization to line 320 as well. This would make it clearer that both flags are reset before triggerPaste is invoked, rather than having one reset before and one reset after the promise is created.
| const triggerPaste = clipboardService.triggerPaste(getActiveWindow().vscodeWindowId); | |
| if (triggerPaste) { | |
| logService.trace('registerExecCommandImpl (triggerPaste defined)'); | |
| PasteOptions.electronBugWorkaroundPasteEventLock = false; | |
| PasteOptions.electronBugWorkaroundPasteEventLock = false; | |
| const triggerPaste = clipboardService.triggerPaste(getActiveWindow().vscodeWindowId); | |
| if (triggerPaste) { | |
| logService.trace('registerExecCommandImpl (triggerPaste defined)'); |
| // Ensure this doesn't run twice, what appears to be happening is | ||
| // triggerPasteis called once but it's handler is called multiple times | ||
| // when it reproduces |
There was a problem hiding this comment.
The comment's wording could be clearer. Consider revising to: "Ensure this doesn't run twice. What appears to be happening is that triggerPaste is called once but its handler is called multiple times when the issue reproduces." This improves readability by fixing the typo, adding proper punctuation, and clarifying the sentence structure.
| // Ensure this doesn't run twice, what appears to be happening is | |
| // triggerPasteis called once but it's handler is called multiple times | |
| // when it reproduces | |
| // Ensure this doesn't run twice. What appears to be happening is that | |
| // triggerPaste is called once but its handler is called multiple times | |
| // when the issue reproduces. |
fixes #284848