Toolshed accept and return type commands for searching through toolshed commands #6251
Conversation
| // TODO: This should be an optional parameter, --generic, instead. | ||
| [CommandImplementation("accepts_generic")] |
There was a problem hiding this comment.
Optional parameters are supported, though not with the --<argName> syntax, so you're forced to either specify them or use an explicit pipe | to chain the output into another command.
If we want the -- syntax, I guess its fine to keep the TODO, but there should probably be an issue for the missing feature.
| var impls = cmd.Cmd.CommandImplementors[cmd.SubCommand ?? string.Empty] | ||
| .MethodsByPipedType(pipedArgument, allowGeneric); | ||
|
|
||
| var anyNonGeneric = false; |
There was a problem hiding this comment.
AFAICT anyNonGeneric is equivalent to fitness >= 1. So IMO it'd be clearer if the bool were just removed, or simply defined using fitness after the foreach block.
Its not really that important, but IMO its a bit more readable.
| fitness = int.Max(fitness, 2); // Exact match | ||
| else | ||
| fitness = int.Max(fitness, 1); |
There was a problem hiding this comment.
why not just
| fitness = int.Max(fitness, 2); // Exact match | |
| else | |
| fitness = int.Max(fitness, 1); | |
| return (cmd, fitness: 2); // Exact match | |
| fitness = 1; |
| if (pipedType == null) | ||
| return false; | ||
|
|
||
| return x.Generic || _toolshed.IsTransformableTo(pipedType, param.ParameterType); |
There was a problem hiding this comment.
This gets rid of the x.Generic check, which is still needed. IsTransformableTo just doesn't support generics atm and currently still needs to bypass it. IIRC generics are basically handled implicitly further down in the try-catch block with
var t = GetGenericTypeFromPiped(pipedType!, x.PipeArg!.ParameterType);
return (x, x.Info.MakeGenericMethod(typeArguments?.Append(t).ToArray() ?? [t]));The way toolshed handles generics is just pretty janky overall, it doesn't really handle generic type constraints at all, it just checks it implicitly by seeing if MakeGenericMethod throws. So I think you either need to shuffle logic around to get the a concrete method & parameter types before calling IsTransformableTo, or you just can't quite do this (its currently causing test fails)
2 participants
This is a patch from @moonheart08
This PR adds four new commands:
cmd:accepts = Filters commands based on what type they accept as a piped argument
cmd:accepts_generic = Filters commands based on what type they accept as a piped argument, allowing commands that broadly accept most types
cmd:returns = Filters commands based on what type they return as a result
cmd:returns_generic = Filters commands based on what type they return as a result, allowing commands that broadly accept most types