Clarify that -all switch excludes prereleases

[AArnott]

The claim of a binary file is false. Here is the diff:

-\n  -all           Finds all instances even if they are incomplete and may not launch.\
+\n  -all           Finds all instances except prereleases, even if they are incomplete and may not launch.\

[heaths]

Given there's already a separate -prerelease switch, how does it improve to duplicate information for -all? They are completely unrelated. -all also doesn't include non-default products (Community, Professional, and Enterprise). In fact, they are already all sandwiched right together:

[AArnott]

The issue I'm trying to fix is that the docs for the -all switch should be self-descriptive. It shouldn't say "all instances" when in fact it does not bring in all instances.
When I'm trying to figure out why the output is empty, even when I specify -all, I shouldn't have to read every other switch to discover that the -all doc is lying to me and doesn't actually include all instances.

As it was, I was becoming convinced that vswhere.exe was just broken because it was not producing any output.

[heaths]

Where do we draw the line? All instances could also be confused to mean all products, which also isn't correct. That's why there's another switch for that. I could instead see changing it to something like "Finds instances in complete, launchable, and incomplete states." and eliminate the word "all", but I don't think it's advantageous to start intermixing purposes of other flags.

[AArnott]

I work here, and even I don't understand what "products" even means in this context. I know what "instances" means (I think) but I wonder how many customers do. I think the docs here should try to use terms that most users will understand. If we have to use terms that mostly only the setup team (and their alumni) understands, we should try to define or clarify where those terms are used.
Where should we draw the line? Where the tool does. The documentation should tell exactly what will be produced and what will not be. Users should not have to read every single switch's documentation in order to understand one switch's behavior. And users shouldn't tend to create scripts that use vswhere.exe that "works on my machine" and fails on another because what vswhere.exe appears to do (e.g. print the location of my one VS installation) doesn't work on another machine where only a prerelease was installed merely because the user didn't realize that -all doesn't mean all. The docs should help the script-writer to get it right the first time, and without testing with all sorts of combinations of VS installations to see what vswhere would do in those situations.

[AArnott]

I'm learning though from your points that my PR may be insufficient at fixing all the usage docs. But I may not be qualified to write all the fixes because I don't understand how the product behaves in every case. I welcome you (@heaths) or any other owner of this code to add to or replace my PR with a more complete fix.

[christineruana]

@heaths - above where you say that -all doesn't include non-default products... what about Build Tools SKU? Is that going to be caught by vswhere -all?

[heaths]

No, -all also does not include Build Tools nor would be, because it'd be breaking. See #8 for a long history of details.

That's my point: I could word -all to maybe not use "all" if that's confusing, but mentioning all the things it doesn't pick up when those conditions are detailed directly below it is a slippery slope. Reading more of the parameters for command line tools may be necessary.

[AArnott]

mentioning all the things it doesn't pick up when those conditions are detailed directly below it is a slippery slope. Reading more of the parameters for command line tools may be necessary.

Then at the very least, add to the -all switch doc that this doesn't include all products/skus/etc. and that users should read the other switches for details. Something to hint to folks that they may need to use other switches to get a larger set of results.
I get that you can't change the name or behavior of the switch at this point. But anything we can do to improve understanding of its behavior in the docs is goodness. Particularly since for most tools, reading the doc for one switch is sufficient to understand that switch, and folks (like me) won't realize they have to read all the other switches to understand what -all doesn't entail.

[christineruana]

Hm. I just recommended to the Defender team that they use VSWhere to find VS installed instances so they can then do some exclude directory magic to improve perf. Looks like this won't work by default for them for Preview SKUs. They may also need to do this for BuildTools SKU too.

@AArnott - would it be possible to adjust your string even further and say this:
Finds all instances of Community, Pro and Enterprise SKUs except prereleases, even if they are incomplete and may not launch.

That way if they are thinking of other SKUs, and BuildTools will be the most common, they can look for additional help on how to do this.

I'll open a bug against our setup team to consider having VSWhere -all actually return Preview and BuildTools SKUs too under the default -all.

Also, as a PM on the setup team, I also still have to periodically do a little mental gymnastics to remind myself that in setup contexts, Product often means SKU.

[heaths]

-all cannot returned Build tools or other SKUs. That's a breaking change that will impact all CIs. They can do what they need using the -prerelease and -products * switches. This is purely about clarifying the documentation - not changing the behavior.

vswhere -products * -prerelease -all -property installationPath

Would get the installation path for all products in all instances in any state. Breaking the behavior of a single parameter is not necessary as this has always been possible.

Though, you really shouldn't be using -all often. If a product isn't launchable, there's a good change you're going to exhibit bad behaviors during build that will be hard to diagnose because the product instance could be broken in any given way and errors manifest in seemingly random ways. -all is generally useful when you are trying to figure out why an instance isn't showing up without -all since vswhere shows the state and whether the instance is launchable.

[heaths]

@AArnott is the following more clear?

Finds instances in a complete, launchable, or incomplete state. By default, only instances in a complete state - no errors or reboot required - are searched.

This PR won't work. Because of how the .rc is encoded, you can't modify it on GitHub. You either need to do it in VS, or I can close this PR and create a new one.

[AArnott]

Finds instances in a complete, launchable, or incomplete state. By default, only instances in a complete state - no errors or reboot required - are searched.

Yes, that's better. I would say that's sufficient given the other switches, but I hesitate because the switch is called -all. I feel given that it's worth it in the documentation to describe what isn't included in "all", or at least to use verbiage to set the scope of what is included (e.g. "Finds non-prerelease instances...").

[heaths]

I did clarify which specific states are included, which is what this v1 parameter was to specify. The other switches were added without breaking functionality as requirements evolved. -all always did have everything to do with instance state.

[AArnott]

-all always did have everything to do with instance state.

I think I see how you see it now. And at least in that mindset, I agree with you that your proposed wording is sufficient.
I just wonder how many customers think about it that way. Or do they see it the way I saw it up to this point, where they interpret all to mean all installations rather than to remove a filter on instance state.

Anyway, your proposal certainly seems like an improvement. Please make it. I'm ok to await further customer feedback if more change is necessary.