Sorting of entries in the refactor preview puts checked items last instead of first

[sean-mcmanus]

• VSCode Version: 1.43.2
• OS Version: Windows or others.

Steps to Reproduce:

1. See issue Better sorting of entries of refactor preview #90664 , that says "We should make sure that categories with unconfirmed changes go atop, files should be sorted by name and changes by line number".

Expected: Our C/C++ extension would prefer the opposite behavior with the confirmed changes going on at the top, which is what was originally requested at #77728 (comment) and what matches Visual Studio's C++ Rename preview:

Was there a reason the opposite sorting was desired? Our Find All References UI (and our current Rename UI) puts the confirmed/checked references at the top, so flipping the order for rename is confusing to our users. In many scenarios, users just want to preview the confirmed/checked references, and they skip the other stuff, which is why we'd prefer the other types of unconfirmed/unchecked references to be at the bottom, since those types are less likely to be renamed.

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

[vscodebot]

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

• Better sorting of entries of refactor preview (#90664)

[sean-mcmanus]

@vscodebot Not a duplicate.

[jrieken]

Was there a reason the opposite sorting was desired?

The idea is that un-confirmed results need human eyes to check wether they should be included or not. For those that the computer is certain no check is needed, hence it is less important. That's the line of thought here. Disagreement?

[bobbrow]

I think part of the concern here is that the panel these results are appearing in is not very tall (by default), so the user is often presented with a bunch of empty check boxes as if the extension couldn't figure out any of the renames on its own. They'd have to scroll down to see that we had confidence about any of the rename candidates. Putting the confirmed references at the top gives users more confidence in our extension's results.

Another option: Put the confirmed ones at the top and collapse the node?

[sean-mcmanus]

This is a visual before and after example of what we're concerned about:

After (confirmed at the bottom):

Before (confirmed at the top):

Yes, unchecking a confirmed reference is unlikely, but to me it’s mostly about seeing/previewing what the rename will do as a 1st step, and then when that looks good I might look at the other stuff I might want to add (i.e. it’s the “read order” that makes sense to me).

However, we could wait till we publish our next Insiders and see if we get any customer feedback on the new UI and sort order.

[bobbrow]

but to me it’s mostly about seeing/previewing what the rename will do as a 1st step, and then when that looks good I might look at the other stuff I might want to add (i.e. it’s the “read order” that makes sense to me).

This makes sense to me. I think the discussion here is whether rename "preview" should prioritize previewing what is about to happen (the confirmed references) or whether it prioritizes previewing what might be skipped by the operation. I agree with @sean-mcmanus that it is about the former.

[sean-mcmanus]

Here's another reason why confirmed/checked references should be shown first for C++ -- 2 different objects of different types sometimes get confirmed as the same reference when they're not.

I filed VS bug https://developercommunity.visualstudio.com/content/problem/1014321/c-intellisense-find-all-references-confirms-refere.html , but I don't believe it's a regression and may be considered a "by design limitation", so I'm not expecting it to be fixed soon.