Fix URI Aware Command Handler Argument Duplication

[colin-grant-work]

Signed-off-by: Colin Grant colin.grant@ericsson.com

What it does

Fixes #8585 by preventing duplication of arguments passed in. Also addresses the todo comment by creating separate classes for mono- and multi-select cases.

There are some remaining questions:

• Given the potential confusion created by the generic and options, it may be desirable to deprecate the old UriAwareCommandHandler - except that it's likely to have been used quite widely.
• Creating separate classes for the mono- and multi-select cases seems a bit heavy. Instead it would be possible to encourage users to instantiate the UriAwareCommandHandler class through the methods UriAwareCommandHandler.MultiSelect and UriAwareCommandHandler.MonoSelect (or better names :)), since then the typing only needs to be correct there.
• This is potentially breaking in two cases:
  1. If anyone was working around it and expecting two copies of the URI.
  2. The old implementation would turn a single URI into [URI] in a multi-select context, but that seems like bad behavior. If the function expects URI[], then it seems like URI is a bad call, not something to be corrected. But if anyone was relying on that, they would now get an unexpected result.

How to test

1. Run the tests.
2. Copy the snippet from URIAwareCommand Handler Duplicates URI Arguments #8585 into packages/core/src/browser/common-frontend-contribution.ts in the registerCommands function, build, and run the 'Will Cause an Error' command. See that it doesn't cause an error anymore.

1. Use some UriAwareCommandHandlers and see that they work (e.g. right click on a folder in the file explorer and try 'Find in Folder')

Review checklist

• as an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• as a reviewer, I agree to behave in accordance with the review guidelines

[paul-marechal]

Good changes and ideas!

My opinion about the points you brought up (clients depending on the URI clone bug, and the multi state typing bug) is that we should deprecate the old UriAwareCommandHandler while implementing the new ones which are free of bugs.

Deprecating doesn't mean removing, so mid-term it should be fine. Until we actually remove any deprecated API for good (on that note I did a test PR for this and I still need to take care of it at some point).

Now no matter how we handle the implementation for single vs multi uri selections, I agree that a simple API for clients is preferable. I feel like what you did with a namespace here is fine.

[colin-grant-work]

@marechal-p, I've removed all calls (I think) to the UriAwareCommandHandler constructor in the repo. There are a couple of methods in different places that just duplicate the functionality of the new UriAwareCommandHandler.MonoSelect/MultiSelect, but I've left those in place, in case adopters refer to those methods in derived classes.