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

fix updating items in the quick-pick menu #10055

Closed
wants to merge 2 commits into from
Closed

fix updating items in the quick-pick menu #10055

wants to merge 2 commits into from

Conversation

vinokurig
Copy link
Contributor

@vinokurig vinokurig commented Sep 6, 2021

Signed-off-by: Igor Vinokur ivinokur@redhat.com

What it does

Fixes a bug when quick pick items are not updated on the fly.

When a quick open widget is shown, set an instance of the monaco quick-input to the sessions map.
When plugin request's a quick-input items update, they are set to the monaco quick-input instance from the map:

} else if (param === 'items') {
handlesToItems.clear();
params[param].forEach((item: TransferQuickPickItems) => {
handlesToItems.set(item.handle, item);
});
(input as any)[param] = params[param];

How to test

  1. Clone and compile the test plugin: https://github.com/microsoft/vscode-extension-samples/tree/main/quickinput-sample
  2. Start hosted plugin
  3. Execute the Quick Input Samples from the command palette.
  4. Choose quickOpen and type something to the input box.

Review checklist

Reminder for reviewers

@tsmaeder
Copy link
Contributor

tsmaeder commented Sep 6, 2021

@vinokurig can you describe what the fix does in the code? I.e. what is that basic idea of the code changes in the fix? Also, what was the cause of the bug? Remember to "write shit down" (TM)

Signed-off-by: Igor Vinokur <ivinokur@redhat.com>
@vinokurig
Copy link
Contributor Author

@tsmaeder

@vinokurig can you describe what the fix does in the code? I.e. what is that basic idea of the code changes in the fix? Also, what was the cause of the bug? Remember to "write shit down" (TM)

Updated the description

@tsmaeder
Copy link
Contributor

tsmaeder commented Sep 7, 2021

@vinokurig I think your change takes the code in the wrong direction: instead of adding "currentInput" state into the QuickInputService, we should add a "createQuickPick" method to the interface and use that in the quick-open-main.ts#$showCustomQuickPick. The current change fails when someone calls QuickInputService#showQuickPick twice in a row. There have been a number of things wrong with this area of the code for a long time which are not your fault, but we should refrain from making the situation worse.

Copy link
Contributor

@tsmaeder tsmaeder left a comment

Choose a reason for hiding this comment

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

See comment on the main page for the reasons.

@vinokurig
Copy link
Contributor Author

Opened a PR with different approach #10065

@vinokurig vinokurig closed this Sep 8, 2021
@vinokurig vinokurig deleted the che-20316 branch September 14, 2021 15:21
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.

2 participants