Extend Extension API with QuickPickSeparator Support

[go2sh]

With #21244 QuickPick Controls got the support for separators. This functionality is currently not available through the extensions API. For a nice look and feel, it would be great to make the separators available for extensions.

It seams to be not to hard to implement and I'am willing to do it, but I'am not sure from an API standpoint. Would you accept such a change?

The API could be changed in to ways:

• Add a property to the QuickPickItem to show a seperator above, add this seperator in the main thread.
• Add a Interface QuickPickSeparator to the extension api and change all occurrences of items to (T | QuickPickSeparator)[] or ReadonlyArray<T | QuickPickSeparator>

[chrmarti]

Adding an interface similar to the internal one would make sense:

interface QuickPickSeparator {
	type: 'separator';
	label?: string;
}

PR welcome. When adding QuickPickSeparator to the existing API, we will need to make sure we don't break existing clients.

/fyi @jrieken

[jrieken]

PR welcome

Not sure, API via PRs are always hairy because API gets discussed a lot.

[go2sh]

@jrieken @chrmarti I create a draft for the API changes: go2sh@b49406c
Feel free to have a look and discuss it.

I'am also almost done with the changes for the extension host components.

[go2sh]

Okay I'am done. I found two things to consider:

There are some vscode internal extensions, which use the name type already for a different usage. So it might conflict and thus break some stuff. How do you handle this case?

The type of activeItems and selectedItems is hard. If it's just T code like in the tests:

pick.activeItems = [pick.items[0]];

will break. To avoid this, the type could be T | QuickPickSeperator and the value could be filtered on setting.

Maybe Using a flag for the separator might be less destructive.

[eamodio]

To me, adding a new property to the existing QuickPickItem is an easier way to go

Something like

export interface QuickPickItem {
	...
	beginGroup?: boolean;
}

or

export interface QuickPickItem {
	...
	beginGroup?: boolean | string;
}

If you want to be able to put a label on the separator

[jrieken]

👏 I like that much better @eamodio. Maybe with "begin" but just "group" or "groupInformation"

[chrmarti]

We had it as a property on the items in our internal API and it led to complicated code when using it because some otherwise random item suddenly also represents a group of items. That is why I introduced the QuickPickSeparator interface when doing the new implementation.

[eamodio]

The separator method, seems like it would be problematic with search in the quickpick (or do you just throw away all separators in that case?)

@jrieken with this:

export interface QuickPickItem {
	...
	group?: string;
}

Would you expect it would still behave as a beginning marker, or an actual grouping construct? While a grouping construct might be nice, it feels like it makes things harder on the QuickPick side if the items in a group aren't consecutive. Although completely out of scope here -- have a true grouping construct, like QuickPickGroup that also supported items (and ideally a promise of items), could be a great way of having async groups in the quickpick. For example, in some GitLens quick pick the commands are static, but other parts could be dynamically provided. But I'm getting out of scope for this 😄

[chrmarti]

Search rearranges the items according to how well they match, so items are no longer in the original groups. Because of that we remove the separators. (See, e.g., the theme picker.)

[Zingam]

Is this still not implemented? I work on a feature for the CMake extension, which would really benefit of having an item separator to separate the toolchain kits from an action to rescan the system for kits.

[alefragnani]

Some comments from my #11468 (closed yesterday as #duplicate in favor of this one), which I think are relevant to be present here

---

Original request

The idea is to show items in the pick list, just like Go to Symbol, where you have some kind of groups of items (that appears when you type '@:' in the pick list)?

---

More details added 2 days ago

I was thinking about a group/category data attached to each QuickPickItem and the QuickPick itself would deal with grouping and counting the items in each group (when properly turned on with a new QuickPickOptions option). At first sight, I thought the groups (line separator and labels/counting) would be generated automatically.

But, if the proposed API (QuickPickItem and QuickPickSeparator) could give me the same power/flexibility as we see today in Go to Symbol command, it would be great.

I just wonder how/if I should deal with the ordering, visibility and counting myself when the user filters the PickList, with the QuickPickSeparator. As you see, the counter changes, so should I "recreate/update" that QuickPickSeparator too?

I didn't dig in VS Code source to see how the internal API works today, but with the right documentation and samples, I guess it wouldn't be a problem.

---

Thank you

[eamodio]

How about something like this:

export interface QuickPick {
	...
	groups?: QuickPickItemGroup[];
}

export interface QuickPickItemGroup {
	id: string;
	label?: string;
	options?: { showCount?: boolean }; // probably don't need to support this in v1
}

export interface QuickPickItem {
	...
	group?: string;
}

Where you could specify an ordered array of groups, with optional labels, and then apply the group id to each item via the group property. If groups isn't used or a group doesn't exist, then it just falls back the to "default group" which would always be the last "group"

This somewhat follows our menu pattern (with the exception of the defined groups, since we want to apply labels/other visual indicators.

/cc @chrmarti @jrieken

[jrieken]

That seems very complicated. Given that a group is just a label, it could be just this

export interface QuickPickItem {
	group?: string; // or groupLabel, groupName, category, etc
}

[eamodio]

Well without specified groups there wouldn't be a controlled ordering (especially if the label is also it's identifier). Nor the ability to have label-less groups (just a separator really).

[jrieken]

I believe the order is always as it is defined inside the array and when we re-order we don't show groups. For groups without labels - I am not sure that's desirable.

[connor4312]

I like the groups property approach, but that doesn't allow arbitrary/unnamed separators. In theory I could have an empty string group, but that would be the only one of that group I could have.

[TylerLeonhardt]

So with the discussion in API sync, I'm proposing:

	export interface QuickPickItem {
		kind?: string | { label: string; };
	}

• The kind aligns nicely with our other APIs.
• (not sure if needed right now...) The extra { label: string; } allows for future proofing of separators if we ever introduce like (color, a click listener, etc)
• No breaking changes needed whatsoever

an example:

const a = await window.showQuickPick([
		{
			label: 'asdf',
			kind: 'asdf',

		},
		{
			label: 'asdf',
			kind: 'asdf'
		},
		{
			label: 'asdf',
			kind: 'asdf'
		},
		{
			label: 'asdf',
			kind: 'asdf'
		},
		{
			label: 'zxcv',
			kind: 'zxcv'
		},
		{
			label: 'zxcv',
			kind: 'zxcv'
		},
		{
			label: 'zxcv',
			kind: 'zxcv'
		},
		{
			label: 'star',
			kind: 'star'
		},
		{
			label: 'idk',
		},
	]);

[TylerLeonhardt]

My conclusion is that this is very easy to use for the extension author and I was able to add these separators to existing quick picks with little effort.

I like the groups property approach, but that doesn't allow arbitrary/unnamed separators. In theory I could have an empty string group, but that would be the only one of that group I could have.

I missed this comment before and it brings up a good point... how would we add an arbitrary separator between two groups?

If we still encapsulate this in within kind I wonder if we could leverage the extra { label?: string } bag somehow to say "hey this is the start"

[TylerLeonhardt]

So in standup @connor4312 and @Tyriar convinced me that kind is harder to use than a separator...

@connor4312 pointed out that the kind proposal for quickpick separators isn’t ideal because it would not be possible to have a single blank line separating two groups.

and @Tyriar pointed out that kind is ambiguous if you have a situation like this:

kind: ‘a’
kind: ‘b’
kind: ‘a’

we discussed this in the API sync as ‘doc-able’ but I do share the same feeling as he.

Further discussion with @jrieken left us with:

enum QuickPickItemKind {
  Default, 
  Separator
}

interface QuickPickItem {
  // when set to Separator the following properties are ignored....
  kind?: QuickPickItemKind
}

So that we didn't need a separate type just for separators and so we didn't use string enums or literal or types.

However, we can't go down that route because QuickPickItem#label can't be undefined... where in separators in can...

I also wanted to call out that @chrmarti mentioned a tree-like structure... which might have looked similar to what @connor4312 was suggesting here:
#74967 (comment)

[TylerLeonhardt]

However, we can't go down that route because QuickPickItem#label can't be undefined... where in separators in can...

Going down this route... where QuickPickItem#label can just be empty string

[TylerLeonhardt]

Folks were happy in the API call so let's see how people feel about the API.

[TylerLeonhardt]

Please take a look at the example in #137745 and don't be afraid to provide feedback here. Moving this to December to track finalization.

[TylerLeonhardt]

Finalized in e15397d and eb416b4