Add DescriptionMethodAttribute for dynamic help text generation… #513
Conversation
|
|
||
| if (!method.IsStatic) | ||
| { | ||
| return $"Error: Method '{methodName}' must be static"; |
There was a problem hiding this comment.
Is it possible to reach this line? I think with BindingFlags.Static in the filter, method will be null. Perhaps add a hint to the not found message.
|
|
||
| // Invoke the method | ||
| var result = method.Invoke(null, null) as string; | ||
| return result; |
|
|
||
| if (method == null) | ||
| { | ||
| return $"Error: Method '{methodName}' not found in type '{declaringType.Name}'"; |
There was a problem hiding this comment.
Given this is returning a string, will the string be included in the help output the end user sees? I would expect this to throw an InvalidConfigurationException at some point so devs are warned early when there is an error.
Although, now that I say that, if this is only discovered when help is run for a command, then it's all the more likely a misconfiguration will escape detection. Perhaps we should resolve the method (but not execute it) in the DefinitionMappingExtensions, setting it in a new argument.DescriptionMethod property. The other benefit is that we'll no longer need the "DESCRIPTION_METHOD" magic string. You'll be able to check if DescriptionMethod is null.
Once that's done, you could hide the complexity of this behind argument. the argument.Description property could return an internal field value or execute the DescriptionMethod if it exists. Any place that calls argument.Description could take advantage of the feature.
|
Hi @cimdo. Thanks for your contribution. This is a feature that has been asked for. I'm in the middle of a move so I won't be able to fully review until next week. At first glance, it looks good. I added some comments. The feature is looking good. It will need documentation as well. I'm happy to help with that if it's not obvious how to do it. |
|
@cimdo After review, I see two outstanding tasks to merge this PR
If you're not sure how to do #2, I'm happy to help once you've implement the changes for #1 Cheers, |
Add DescriptionMethodAttribute for dynamic help text generation
Resolved #472
Summary
This PR implements a new
DescriptionMethodAttributethat allows developers to specify methods for generating dynamic help descriptions at runtime, rather than being limited to static descriptions.