Adding IArgumentCompleterFactory for parameterized ArgumentCompleters #12605

powercode · 2020-05-07T21:54:05Z

PR Summary

Adding support for derived, parameterized, ArgumentCompleterAttributes. This makes it possible to create more generic completers, either in libraries or shipping with the product.

This allows for parameterized ArgumentCompleters.
Deriving from ArgumentCompleterAttribute makes it possible to create generic completers, like

[DirectoryCompleter(ContainingFile="pswh.exe", Depth=2)]

[DateCompleter(WeekDay='Monday', From="LastYear")]

[GitCommits(Branch='release')]

The derived attributes implement the IArgumentCompleterFactory interface and use property values to create a specialized completer.

PR Context

It is difficult (if even possible) to write generic completers. This PR would allow for the following:

   /// <summary>
    /// Creates new number completions
    /// </summary>
    public class NumberCompletionsAttribute : ArgumentCompleterAttribute, IArgumentCompleterFactory
    {
        private readonly int _from;
        private readonly int _to;
        private readonly int _step;

        /// <summary>
        /// Creates a new <see cref="NumberCompletionsAttribute"/>.
        /// </summary>
        /// <param name="from"></param>
        /// <param name="to"></param>
        /// <param name="step"></param>
        public NumberCompletionsAttribute(int from, int to, int step = 1) 
        {
            _from = @from;
            _to = to;
            _step = step;
        }

        /// <inheritdoc />
        public IArgumentCompleter Create() => new NumberCompleter(_from, _to, _step);
    }

    /// <summary>
    /// Completes numbers
    /// </summary>
    public class NumberCompleter : IArgumentCompleter
    {
        private readonly int _from;
        private readonly int _to;
        private readonly int _step;

        /// <summary>
        /// Creates a new instance of a NumberCompleter
        /// </summary>
        /// <param name="from"></param>
        /// <param name="to"></param>
        /// <param name="step"></param>
        public NumberCompleter(int from, int to, int step)
        {
            _from = @from;
            _to = to;
            _step = step;
        }

        /// <inheritdoc />
        public IEnumerable<CompletionResult> CompleteArgument(string commandName, string parameterName, string wordToComplete, CommandAst commandAst, IDictionary fakeBoundParameters)
        {
            var res = new List<CompletionResult>((_to - _from) / _step);

            for (int i = _from; i <= _to; i += _step)
            {
                string completionText = i.ToString();
                if (completionText.StartsWith(wordToComplete, StringComparison.Ordinal))
                    res.Add(new CompletionResult(completionText));
            }

            return res;
        }
    }

or in PowerShell:

using namespace System.Management.Automation
using namespace System.Management.Automation.Language
using namespace System.Collections
using namespace System.Collections.Generic


class NumberCompleter : IArgumentCompleter {

    [int] $From
    [int] $To
    [int] $Step
    
    NumberCompleter([int] $from, [int] $to, [int] $step) {
        if ($from -gt $to) {
            throw [ArgumentOutOfRangeException]::new("from")
        }
        $this.From = $from
        $this.To = $to
        $this.Step = $step -lt 1 ? 1 : $step
    }

    [IEnumerable[CompletionResult]] CompleteArgument(
        [string] $CommandName,
        [string] $parameterName,
        [string] $wordToComplete,
        [CommandAst] $commandAst,
        [IDictionary] $fakeBoundParameters) {
        
        $resultList = [List[CompletionResult]]::new()
        $local:to = $this.To
        $local:step = $this.Step
        for ($i = $this.From; $i -lt $to; $i += $step) {
            $resultList.Add([CompletionResult]::new($i.ToString()))
        }
        
        return $resultList
    }
}

class NumberCompletionAttribute : ArgumentCompleterAttribute, IArgumentCompleterFactory {
    [int] $From
    [int] $To
    [int] $Step
    
    NumberCompletionAttribute([int] $from, [int] $to, [int] $step) {
        $this.From = $from
        $this.To = $to
        $this.Step = $step
    }

    [IArgumentCompleter] Create() { return [NumberCompleter]::new($this.From, $this.To, $this.Step) }
}

Usage from PowerShell would then be:

function Add{
    param(
       [NumberCompletions(0, 100, 5)]
       [int] $X
      ,
       [NumberCompletions(0, 100, 5)]
       [int] $Y
    )
    $X + $Y
}

Fix #12708

PR Checklist

PowerShell.sln.DotSettings

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs

powercode · 2020-05-08T11:04:09Z

The CI errors seem unrelated to my commits

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs

powercode · 2020-05-08T16:42:11Z

Yet again an unrelated CI failure

iSazonov · 2020-05-08T17:21:06Z

Add PowerShell Committee to approve new public API.

iSazonov · 2020-05-12T09:28:28Z

Do you think about IArgumentCompleter vs IArgumentCompleterFactory?

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs

joeyaiello · 2020-05-13T22:50:25Z

@PowerShell/powershell-committee discussed this and we all think it's super interesting, and, at least in our initial look, a well thought out feature.

However, given it's scope, it'd be really nice to see an RFC (or even an issue) where the problem statement and any alternative solutions are discussed and other contributors have a chance to give feedback on the proposal.

@JamesWTruher also suggested that it might be useful to see a real-world example of how this could be used to refactor PowerShell built-in completers, or to make them more intelligent. E.g. Import-Module -Path <tab> might only show folders, *.psd1 files, and *.psm1 files, and that could be reused in other places where it's relevant.

iSazonov · 2020-05-14T05:35:18Z

Convert to Draft until RFC or issue discussion is finished.

daxian-dbw · 2020-05-14T05:40:06Z

I like the idea, very interesting, allowing people to have reusable argument completer attributes. About the implementation, why not have an abstract attribute class that defines an abstract method which returns an IArgumentCompleter instance? For example:

[AttributeUsage(AttributeTargets.Field | AttributeTargets.Property)]
public abstract class ReusableArgumentCompletionAttribute : Attribute
{
    protected ReusableArgumentCompletionAttribute() {}
    public abstract IArgumentCompleter NewArgumentCompleter(/* EngineIntrinsics engineIntrinsics ?? */);
}

This seems more succinct than : ArgumentCompleterAttribute, IArgumentCompleterFactory

SteveL-MSFT · 2020-05-19T20:22:07Z

8000

The @PowerShell/powershell-committee discussed this. In general, we all agree this capability would be useful so thanks for that! However, we're trying to update our RFC process to make it lighter-weight and would like to try it out with this feature if you're ok with this. Basically, we'd like to have an issue opened that describes the problem to be solved and/or value to the user and also some example code showing its use (which you have in this PR description already) and expected results. Based on the amount and quality of discussion, it will help inform if an RFC is needed.

vexx32 · 2020-05-19T20:44:37Z

@SteveL-MSFT in case y'all missed it. 🙂
#12708

SteveL-MSFT · 2020-05-20T16:20:06Z

@vexx32 yes, we missed that. Thanks! I linked it to this PR.

ghost · 2020-05-28T02:00:13Z

8000

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Mainainer, Please provide feedback and/or mark it as Waiting on Author

ghost · 2020-12-01T02:00:07Z

This pull request has been automatically marked as Review Needed because it has been there has not been any activity for 7 days.
Maintainer, please provide feedback and/or mark it as Waiting on Author

ghost · 2020-12-09T23:52:44Z

Open PRs should not be assigned to milestone, so they are not assigned to the wrong milestone after they are merged. For backport consideration, use a backport label.

powercode · 2020-12-14T09:09:43Z

@SteveL-MSFT This PR is in a Change Requested state, but the requested change is on a commit that is no longer available due to a force push after a rebase, and the requested change is made. How can I get this into a green state?

powercode · 2021-01-18T13:45:09Z

@adityapatwardhan It's been 8 months since you last touched this. Can you act on it or maybe reassign it if you don't have the bandwidth to process it?

powercode · 2021-01-27T12:29:46Z

@iSazonov I'm suspecting that this is in limbo as there is 1 change requested, but it is requested on a commit that isn't available anymore, and hence cannot be fixed. The requested change is already in a different, force-pushed commit.

What do you think is the right way to move this forward? This is actually a very nice feature to have, that opens up new possibilities, both for the team and for regular users, and it's a shame that it's just stuck here.

adityapatwardhan · 2021-03-25T22:24:14Z

Sorry for the delay. I have reviewed this and looks good. Since it has been a while since we ran the CI tests for this PR. I am restarting all the tests. I will merge as soon as the tests pass.

adityapatwardhan · 2021-03-25T22:25:43Z

Closing and re-opening to re-trigger all CI tests

adityapatwardhan · 2021-03-25T22:26:04Z

@PoshChan please remind me in 1 hour

PoshChan · 2021-03-25T23:27:07Z

@adityapatwardhan, this is the reminder you requested 1 hour ago

adityapatwardhan · 2021-03-26T00:07:04Z

@powercode Thank you for your contribution and patience!

iSazonov · 2021-03-26T05:33:00Z

@powercode Please open new documentation issue.

/cc @sdwheeler

sdwheeler · 2021-03-29T15:48:13Z

@powercode @iSazonov The scripting example can be added to the docs. The C# usage needs to be added in ///-comments in the source code. Please open the appropriate documentation needed issue in this repo.

ghost · 2021-04-14T23:59:41Z

🎉v7.2.0-preview.5 has been released which incorporates this pull request.:tada:

Handy links:

Release Notes

@davidBar-On

[7.2.0-preview.5] - 2021-04-14 * Breaking Changes - Make PowerShell Linux deb and RPM packages universal (#15109) - Enforce AppLocker Deny configuration before Execution Policy Bypass configuration (#15035) - Disallow mixed dash and slash in command line parameter prefix (#15142) (Thanks @davidBar-On!) * Experimental Features - `PSNativeCommandArgumentPassing`: Use `ArgumentList` for native executable invocation (breaking change) (#14692) * Engine Updates and Fixes - Add `IArgumentCompleterFactory` for parameterized `ArgumentCompleters` (#12605) (Thanks @powercode!) * General Cmdlet Updates and Fixes - Fix SSH remoting connection never finishing with misconfigured endpoint (#15175) - Respect `TERM` and `NO_COLOR` environment variables for `$PSStyle` rendering (#14969) - Use `ProgressView.Classic` when Virtual Terminal is not supported (#15048) - Fix `Get-Counter` issue with `-Computer` parameter (#15166) (Thanks @krishnayalavarthi!) - Fix redundant iteration while splitting lines (#14851) (Thanks @hez2010!) - Enhance `Remove-Item -Recurse` to work with OneDrive (#14902) (Thanks @iSazonov!) - Change minimum depth to 0 for `ConvertTo-Json` (#14830) (Thanks @kvprasoon!) - Allow `Set-Clipboard` to accept empty string (#14579) - Turn on and off `DECCKM` to modify keyboard mode for Unix native commands to work correctly (#14943) - Fall back to `CopyAndDelete()` when `MoveTo()` fails due to an `IOException` (#15077) * Code Cleanup <details> <summary> We thank the following contributors! @xtqqczze, @iSazonov, @ZhiZe-ZG </summary> <ul> <li>Update .NET to <code>6.0.0-preview.3</code> (#15221)</li> <li>Add space before comma to hosting test to fix error reported by <code>SA1001</code> (#15224)</li> <li>Add <code>SecureStringHelper.FromPlainTextString</code> helper method for efficient secure string creation (#14124) (Thanks @xtqqczze!)</li> <li>Use static lambda keyword (#15154) (Thanks @iSazonov!)</li> <li>Remo D8DD ve unnecessary <code>Array</code> -> <code>List</code> -> <code>Array</code> conversion in <code>ProcessBaseCommand.AllProcesses</code> (#15052) (Thanks @xtqqczze!)</li> <li>Standardize grammar comments in Parser.cs (#15114) (Thanks @ZhiZe-ZG!)</li> <li>Enable <code>SA1001</code>: Commas should be spaced correctly (#14171) (Thanks @xtqqczze!)</li> <li>Refactor <code>MultipleServiceCommandBase.AllServices</code> (#15053) (Thanks @xtqqczze!)</li> </ul> </details> * Tools - Use Unix line endings for shell scripts (#15180) (Thanks @xtqqczze!) * Tests - Add the missing tag in Host Utilities tests (#14983) - Update `copy-props` version in `package.json` (#15124) * Build and Packaging Improvements <details> <summary> We thank the following contributors! @JustinGrote </summary> <ul> <li>Fix <code>yarn-lock</code> for <code>copy-props</code> (#15225)</li> <li>Make package validation regex accept universal Linux packages (#15226)</li> <li>Bump NJsonSchema from 10.4.0 to 10.4.1 (#15190)</li> <li>Make MSI and EXE signing always copy to fix daily build (#15191)</li> <li>Sign internals of EXE package so that it works correctly when signed (#15132)</li> <li>Bump Microsoft.NET.Test.Sdk from 16.9.1 to 16.9.4 (#15141)</li> <li>Update daily release tag format to work with new Microsoft Update work (#15164)</li> <li>Feature: Add Ubuntu 20.04 Support to install-powershell.sh (#15095) (Thanks @JustinGrote!)</li> <li>Treat rebuild branches like release branches (#15099)</li> <li>Update WiX to 3.11.2 (#15097)</li> <li>Bump NJsonSchema from 10.3.11 to 10.4.0 (#15092)</li> <li>Allow patching of preview releases (#15074)</li> <li>Bump Newtonsoft.Json from 12.0.3 to 13.0.1 (#15084, #15085)</li> <li>Update the <code>minSize</code> build package filter to be explicit (#15055)</li> <li>Bump NJsonSchema from 10.3.10 to 10.3.11 (#14965)</li> </ul> </details> * Documentation and Help Content - Merge `7.2.0-preview.4` changes to master (#15056) - Update `README` and `metadata.json` (#15046) - Fix broken links for `dotnet` CLI (#14937) [7.2.0-preview.5]: v7.2.0-preview.4...v7.2.0-preview.5

ghost assigned adityapatwardhan May 7, 2020

PoshChan reviewed May 7, 2020

View reviewed changes

PowerShell.sln.DotSettings Outdated Show resolved Hide resolved

iSazonov reviewed May 8, 2020

View reviewed changes

PoshChan reviewed May 8, 2020

View reviewed changes

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Show resolved Hide resolved

PoshChan reviewed May 8, 2020

View reviewed changes

src/System.Management.Automation/engine/CommandCompletion/CompletionCompleters.cs Show resolved Hide resolved

iSazonov reviewed May 8, 2020

View reviewed changes

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs Outdated Show resolved Hide resolved

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs Outdated Show resolved Hide resolved

iSazonov requested review from SteveL-MSFT and daxian-dbw May 8, 2020 17:19

iSazonov added the CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log label May 8, 2020

iSazonov added this to the 7.1.0-preview.3 milestone May 8, 2020

iSazonov added the Review - Committee The PR/Issue needs a review from the PowerShell Committee label May 8, 2020

iSazonov reviewed May 12, 2020

View reviewed changes

src/System.Management.Automation/engine/CommandCompletion/ExtensibleCompletion.cs Outdated Show resolved Hide resolved

iSazonov marked this pull request as draft May 14, 2020 05:33

TravisEz13 modified the milestones: 7.1.0-preview.3, 7.1.0-preview.4 May 14, 2020

powercode mentioned this pull request May 18, 2020

Support parameterized argument completers #12708

Closed

powercode force-pushed the Completion/DerivedAttributes branch 2 times, most recently from a307843 to 792209e Compare May 18, 2020 08:32

ghost added the Review - Needed The PR is being reviewed label May 28, 2020

SteveL-MSFT added this to the 7.2-Consider milestone Dec 9, 2020

ghost removed this from the 7.2-Consider milestone Dec 9, 2020

SteveL-MSFT added the Backport-7.2.0-Consider label Dec 10, 2020

adityapatwardhan approved these changes Mar 25, 2021

View reviewed changes

ghost removed the Review - Needed The PR is being reviewed label Mar 25, 2021

adityapatwardhan closed this Mar 25, 2021

adityapatwardhan reopened this Mar 25, 2021

Fix codefactor issues

08be643

adityapatwardhan merged commit be7d366 into PowerShell:master Mar 26, 2021

adityapatwardhan added CL-Engine Indicates that a PR should be marked as an engine change in the Change Log and removed CL-General Indicates that a PR should be marked as a general cmdlet change in the Change Log labels Mar 26, 2021

adityapatwardhan added this to the 7.2.0-preview.5 milestone Mar 26, 2021

iSazonov added the Documentation Needed in this repo Documentation is needed in this repo label Mar 26, 2021

powercode mentioned this pull request Mar 29, 2021

Document IArgumentCompleterFactory and parameterized ArgumentCompleters MicrosoftDocs/PowerShell-Docs#7390

Closed

powercode deleted the Completion/DerivedAttributes branch April 16, 2021 12:10

adityapatwardhan removed the Backport-7.2.x-Consider label Sep 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding IArgumentCompleterFactory for parameterized ArgumentCompleters #12605

Adding IArgumentCompleterFactory for parameterized ArgumentCompleters #12605

Adding IArgumentCompleterFactory for parameterized ArgumentCompleters #12605

Adding IArgumentCompleterFactory for parameterized ArgumentCompleters #12605

Conversation

PR Summary

PR Context

PR Checklist