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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assigning undefined to commentThread.contextValue doesn't reset the backing context key #120680

Closed
hunterknepshield opened this issue Apr 7, 2021 · 7 comments
Assignees
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Milestone

Comments

@hunterknepshield
Copy link

hunterknepshield commented Apr 7, 2021

  • VS Code Version: 1.54.3 (commit 2b9aebd)
  • OS Version: macOS 11.3 Beta

Steps to Reproduce:

  1. From package.json, have a menu contribution registered for comments/commentThread/context to show some command with "when": "commentThread != sentinelForPackageJson"
  2. From extension, call commentController.createCommentThread
  3. Leave the thread's contextValue unassigned for now
  4. Observe that the command from package.json is currently visible in a comment thread; type a reply and click the button for that command
  5. Within the handling code for that command, assign the thread's contextValue to "sentinelForPackageJson" (similar to the example extension in microsoft/vscode-extension-samples/blob/main/comment-sample/src/extension.ts when handling mywiki.startDraft). My extension also sets the thread's canReply value to false at the same time.
  6. Have another command to effectively discard the comment created with the prior command. It will set the thread's canReply value to true and set its contextValue to undefined. Observe that in this case, the command from 1 is not rendered, but the textbox to type a reply has reappeared.

I've been playing around with it, and if instead of setting the contextValue to undefined in step 6, I set some other random arbitrary value, then the context key is updated correctly and the command does reappear. I believe it has something to do with the slight difference in commentThreadWidget.ts, specifically around _commentThreadContextValue:

		if (this._commentThread.contextValue) {
			this._commentThreadContextValue.set(this._commentThread.contextValue);
		} else {
			this._commentThreadContextValue.reset();
		}

Using the inspect context keys tool, the commentThread context key is still set to "sentinelForPackageJson", even though the object's property is definitely undefined after the command in 6 runs. For some reason, it seems the lifecycle of the comment thread is latching on to "sentinelForPackageJson" as the default value, even though it wasn't assigned anywhere near the thread's creation time, and was only done so later after user action had taken place + the thread was updated to append a new comment.

Does this issue occur when all extensions are disabled?: No

This is an extension I've been writing.

@vscodebot
Copy link

vscodebot bot commented Apr 7, 2021

(Experimental duplicate detection)
Thanks for submitting this issue. Please also check if it is already covered by an existing one, like:

@rebornix rebornix added the comments Comments Provider/Widget/Panel issues label Apr 8, 2021
@RMacfarlane RMacfarlane added the bug Issue identified by VS Code Team member as probable bug label May 6, 2021
@RMacfarlane RMacfarlane added this to the Backlog milestone May 6, 2021
@RMacfarlane RMacfarlane removed their assignment May 12, 2021
@rzhao271 rzhao271 modified the milestones: Backlog, October 2021 Oct 26, 2021
@jrieken jrieken added the author-verification-requested Issues potentially verifiable by issue author label Oct 28, 2021
@roblourens roblourens added the verified Verification succeeded label Oct 28, 2021
@roblourens
Copy link
Member

Seems like I can still repro this. If I set contextValue to undefined in the second command, the button doesn't show up again, but if I set it to some value, it does show up again.

image

@roblourens roblourens reopened this Oct 28, 2021
@roblourens roblourens added verification-found Issue verification failed and removed verified Verification succeeded labels Oct 28, 2021
@rebornix
Copy link
Member

@roblourens can you check the context keys for the buttons and share the value of commentThread? Also can you share your code snippet?

@rebornix rebornix modified the milestones: October 2021, November 2021 Oct 28, 2021
@roblourens
Copy link
Member

I added a second button so I can get the context of the menu, and it seems like sentinelForPackageJson is not set after running through the steps. Code looks roughly like this

	let thread: vscode.CommentThread;
	context.subscriptions.push(vscode.commands.registerCommand('extension.helloWorld', async (arg1) => {
		const commentController = vscode.comments.createCommentController(
			'id',
			'label'
		);
		const comment: vscode.Comment = {
			body: 'body',
			author: {
				name: 'name',
			},
			mode: vscode.CommentMode.Preview
		};
		thread = commentController.createCommentThread(vscode.window.activeTextEditor.document.uri, new vscode.Range(new vscode.Position(0, 0), new vscode.Position(0, 0)), [comment]);
		thread.contextValue = 'sentinelForPackageJson';
		thread.canReply = false;
});

	context.subscriptions.push(vscode.commands.registerCommand('extension.helloWorld2', async (arg1) => {
		thread.canReply = true;
		thread.contextValue = undefined;
	}));

@rebornix
Copy link
Member

rebornix commented Nov 1, 2021

@roblourens I didn't quite follow the issues you ran into. Did you mean that after running "hello World", the context key for the comment thread is not set to sentinelForPackageJson?

@roblourens
Copy link
Member

I run the first command helloWorld, everything is correct, then I run the second command helloWorld2, and the button should show up now that the contextValue has cleared but the button does not show up.

@rebornix rebornix removed the verification-found Issue verification failed label Dec 2, 2021
@roblourens roblourens added the verified Verification succeeded label Dec 3, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Dec 17, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
author-verification-requested Issues potentially verifiable by issue author bug Issue identified by VS Code Team member as probable bug comments Comments Provider/Widget/Panel issues insiders-released Patch has been released in VS Code Insiders verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

7 participants
@roblourens @rebornix @jrieken @RMacfarlane @rzhao271 @hunterknepshield and others