Have createQuickPick no longer sort by label

[pelmers]

Similar to #63050, I wish to avoid the final sort on the quick pick list. I am using the quickpick as a search field, and the items field I set is already sorted by my relevance metric, so I do not want to have VS Code sort the list again . Specifically, I want an option to skip this block:

vscode/src/vs/workbench/browser/parts/quickinput/quickInputList.ts

Lines 518 to 523 in a69d0f8

	if (query) {
	const normalizedSearchValue = query.toLowerCase();
	shownElements.sort((a, b) => {
	return compareEntries(a, b, normalizedSearchValue);
	});
	}

)

[pelmers]

I would like to implement this by adding a property sortByValue to QuickPickOptions on the extension API (true by default) to control this sorting. Does that sound reasonable and would you review such a PR? @chrmarti

[chrmarti]

I wonder if that would be sufficient. What about the highlighting of the search term on the labels? Would you want to turn that off too?

[pelmers]

@chrmarti highlighting the search term's not in the scope of this issue. Could you explain why you brought it up?

I am concerned the sorting case in particular because this sort() call happens in the renderer itself. Since the extension API expects a list of items rather than a set or map, it seems logical to me to let the extension to decide that this list is already sorted and let the renderer save some processing time (especially for large lists).

[chrmarti]

I don't want to add API without understanding the long-term direction and the highlighting, filtering and sorting all play into this.

The QuickPickOptions already have matchOnDescription and matchOnDetail flags and these control filtering and highlighting.

[pelmers]

Thanks for responding! I also noticed that filter/highlight can only be toggled for the description and detail fields, not the label field.

I imagine there was some kind of discussion when the API was originally added about its long-term vision; unfortunately that was before 'Hello Code' so it isn't in public history.

Allowing the extension to decide whether to sort results will let it provide a better experience to the user any time the default metric of 'index of first match' is not the best one.

For another example, an extension might want to keep the list grouped by category, similar to how built-in file search quickpick groups Recently opened, Search results, and Document symbols in that order.

[chrmarti]

We could add matchOnLabel (like we internally already did) to the API and couple that with filtering, highlighting and sorting. (Currently the sorting is only done based on the label.) That would fit nicely with the existing API while not exposing too fine-grained options that are hard to use.

[sergei-dyshel]

After reading the discussion I still fail to understand how we went from optional sorting feature request to discussing filtering options which are not dependent on sorting in any way....

[chrmarti]

When you want to control sorting yourself, you probably also want to control filtering. I don't want to introduce a flag specific to sorting now only to discover that doesn't solve the entire issue.

[pelmers]

Combining filtering/sorting into a single flag seems reasonable to me: if an extension implements its own filter/sort, it can just set all the match properties to false.

However highlighting probably can't be merged into the same flag, unless an extension could provide its own highlight ranges. It would be unfortunate if turning off sorting also requires giving up highlighting of the search query in the results.

[roblourens]

I agree that we should at least consider these things together even if we don't implement everything now. It's all about who controls the presentation of items in the quick pick. Also for example, if an extension has filtered and sorted based on its own algorithm, and vscode adds highlights based on a different algorithm, that might not make sense, so I could imagine a reason in the future to let extensions return highlight ranges too. But I'm guessing the filtering and sorting that you will do will be close enough to how our fuzzy matching logic works that you still want ours to apply.

[chrmarti]

There is also the difficulty of the existing matchOnDescription and matchOnDetail flags controlling filtering and highlighting together. If we add matchOnLabel it should behave in a similar way (and maybe control sorting in addition).

[pelmers]

@chrmarti another option you could consider that would suit my requirements is adding an optional sortText field to the quick pick items, which has precedent in the completion items API:

vscode/src/vs/vscode.d.ts

Lines 3329 to 3334 in fe1258b

	/**
	* A string that should be used when comparing this item
	* with other items. When `falsy` the [label](#CompletionItem.label)
	* is used.
	*/
	sortText?: string;

[letmaik]

My scenario is that I'm querying a web service based on the filter text which returns the first 10 results. I would like to add a final "Load more results..." item (alternatively a button) which loads the next batch. However, the automatic sorting prevents me from doing that as the next 10 results wouldn't appear below the first 10, but in a more or less random order, which would be confusing.

My ask: an option to disable sorting but still have the highlighting enabled, together with using alwaysShow: true on all items to prevent hiding results based on the filter text.

[chrmarti]

@pelmers Could you explain the scenario you need to control sorting for? Why is the built-in sorting not sufficient? We are having offline discussions whether or not we should allow that much control by the extensions over the UX.

[pelmers]

@chrmarti apologies on late reply, I never got a notification message on this thread.

Here's some brief details to expand on the use case I outlined in the issue opener:

• QuickPick is being used to display search results
• The 'label' field contains the line of text which matches the search
• The details field contains the path/line number that contains that line
• My preferred sorting is by (path, line) so results from the same file are grouped together in ascending line order
• The issue is today VS Code re-sorts the list by looking at 'label' first and discards the existing ordering

[Linskeyd]

@chrmarti, any thoughts on @pelmers expanded details above? It'd be great to have an option to skip the default logic for ordering results, maintaining the existing ordering of the provided results.

[chrmarti]

@Linskeyd It's on my list, I just haven't made it there yet.

[chrmarti]

I'll merge the PR as proposed API with the flag for the createQuickPick() API, but will not add it to showQuickPick(). showQuickPick() wouldn't benefit as much from it because it cannot update the list once the UI is shown and there is also value in keeping that part of the API simple while createQuickPick() can cover the advanced cases.

Hope that makes sense. This will still need an API review before we can move it to the stable API.

[pelmers]

ok since it sounds like we still can't skip sorting when the field has input, I'll reopen this issue.

However @TylerLeonhardt please note #54400 is probably more specific to this case so maybe we should close this and reopen that instead.

[Brian-Bartels]

I'm not sure I understand why the filtering operation does any sorting at all. Is that an expected behavior of a filter?

[gjsjohnmurray]

@TylerLeonhardt can this quickPickSortByLabel proposed API get finalized soon? I recently used it in #199473 in the Git extension, which is built-in so doesn't have any problem using proposed APIs. But we extension developers can't use proposed APIs if we want to publish on Marketplace.

[lszomoru]

@TylerLeonhardt, would it be possible to also add this as an option to window.showQuickPick? Thanks!

[TylerLeonhardt]

This iteration, we are planning on removing the sort logic for the bigger createQuickPick API. That would mean no API would be needed, and if you want the existing sort behavior, you can just call .sort. I'll rename the issue accordingly.

[gjsjohnmurray]

@TylerLeonhardt I'm trying to understand how this won't be a breaking change.

[TylerLeonhardt]

Right, yeah it's a change we'd like to make to get a read on how many extensions depend on the existing behavior. Our suspicion is that this existing behavior is hurting extensions more than it helps and thus the behavior of the API should change.

Naturally, this is not an actual API breakage, since no API shape is changing... it's a behavior difference.

There is a chance that such a change would cause issues... if so, we'll re-evaluate.

[simonvanbernem]

Hi,

I'm new to developing for VSCode and am very interested in this API-change (to create a go-to symbol that is closer to the one from sublime text), but seeing that it got put back into the backlog after being part of milestone twice is confusing to me.

Does that mean it won't happen in the near future? Was it decided to not implement this change after all?