PowerShell-RFC
PowerShell-RFC copied to clipboard
RFC for an argument completer base class
I like the idea of a base class to remove the need to have common code for every completer as I've had to write it for my own completers. One consideration is whether it makes sense to have this as an independent nuget assembly so it can be used for different versions of PS rather than the one it gets checked into.
@SteveL-MSFT I like the idea of that, but I think it would open up too many possible dependency conflicts. If you have two modules that each depend on a different version of that completion assembly, you can easily end up only being able to load one of them into a session at a time.
Until there's a way to isolate dependencies like that properly I think it's best left in the core pwsh toolkit, and folks can simply declare a dependency on a minimum PS version as usual without much hassle.
I see a point to put an end solution in separate nuget but it looks weird for base class.
There already is an function in CompletionCompeters
private static bool CompletionRequiresQuotes(string completion, bool escape)
{
// If the tokenizer sees the completion as more than two tokens, or if there is some error, then
// some form of quoting is necessary (if it's a variable, we'd need ${}, filenames would need [], etc.)
Language.Token[] tokens;
ParseError[] errors;
Language.Parser.ParseInput(completion, out tokens, out errors);
char[] charToCheck = escape ? new[] { '$', '[', ']', '`' } : new[] { '$', '`' };
// Expect no errors and 2 tokens (1 is for our completion, the other is eof)
// Or if the completion is a keyword, we ignore the errors
bool requireQuote = !(errors.Length == 0 && tokens.Length == 2);
if ((!requireQuote && tokens[0] is StringToken) ||
(tokens.Length == 2 && (tokens[0].TokenFlags & TokenFlags.Keyword) != 0))
{
requireQuote = false;
var value = tokens[0].Text;
if (value.IndexOfAny(charToCheck) != -1)
requireQuote = true;
}
return requireQuote;
}
This could be reused. The second parameter is named somewhat unfortunate: escape should probably be something like isGlobbingPath.
I have updated the RFC with more robust quoting
The DevEX working group briefly discussed this and we think that the implementation doesn't necessarily have to be a base class, which could lead to other problems like this: https://en.wikipedia.org/wiki/Fragile_base_class Another alternative might be to expose an interface with extension methods or a mix of 'normal' methods and extension methods. @powercode If you are still motivated to go ahead, I suggest to follow up on @rjmholt previous comment and from then on I think should be easier to get the ball rolling.
I'll tinker a bit with it and will get back with an update.