Based on conversations with @jonsequitur, I think we can improve name based lookups in Powderhouse.
Problem
e.g. consider this command line:
myapp --opt1 123 subcommand --opt1 "hello"
If --opt1 is a different symbol then name-based lookup has to make some assumptions that I think lead to some nasty bugs.
Let's assume the first instance is a CliOption and the second is CliOption, I can't write code like this:
var value = parseResult.GetValue<int>("--opt1");
It will behave differently (and in cases throw) based on the command line we're parsing.
The reason the option at an earlier position is sometimes desired is because handlers for multiple subcommands might want to reference that value.
So now common or cross-cutting code can't make a strong assumption that the name-based lookup will always return the expected type to match the generic parameter.
This is why I said this API is ergonomically appealing but has some correctness issues.
Proposed solution
Instead of building the dictionary for the full tree root down, build it to include only the ancestors of the current command.
This is a breaking change in that you can't get default values for cousins by just saying GetValue. I think that is justified because the user did not enter those values.
Rather than put the name lookup dictionary on SymbolResultTree, put it on CommandResult. The logic will be that the CurrentCommandResult is checked first, where it will generally be found. If it is not found, or the type is wrong, check up the ancestor tree.
We will allocate another dictionary or two, but the dictionaries will be smaller as the whole tree won't be traversed, just the CurrentCommand and its ancestors.
This will also allow a different scenario. If both of the --opt1 options are CliOption<string> and the user wants to get the one on an ancestor. If we decide this is a valid scenario (and it looks like one to me), we can add an API where the user includes the name or symbol for the ancestor command, and we start the search there.
Based on conversations with @jonsequitur, I think we can improve name based lookups in Powderhouse.
Problem
Proposed solution
Instead of building the dictionary for the full tree root down, build it to include only the ancestors of the current command.
This is a breaking change in that you can't get default values for cousins by just saying
GetValue. I think that is justified because the user did not enter those values.Rather than put the name lookup dictionary on
SymbolResultTree, put it onCommandResult. The logic will be that theCurrentCommandResultis checked first, where it will generally be found. If it is not found, or the type is wrong, check up the ancestor tree.We will allocate another dictionary or two, but the dictionaries will be smaller as the whole tree won't be traversed, just the
CurrentCommandand its ancestors.This will also allow a different scenario. If both of the --opt1 options are
CliOption<string>and the user wants to get the one on an ancestor. If we decide this is a valid scenario (and it looks like one to me), we can add an API where the user includes the name or symbol for the ancestor command, and we start the search there.