-
Notifications
You must be signed in to change notification settings - Fork 394
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Misc Toolshed tweaks #4990
Misc Toolshed tweaks #4990
Conversation
… into toolshed-tweaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once these are addressed this should be good to go.
Co-authored-by: Moony <[email protected]>
|
||
if (parameters.TryGetValue(subCmd ?? "", out var existing)) | ||
{ | ||
if (!existing.SequenceEqual(existing)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was previously bugged because existing.SequenceEqual(existing)
is always true. After I fixed it, many commands started erroring so I though the feature was implemented, but I misunderstood it.
If I understand correctly now, it works as long as the (sub)command name & piped type are unique, so I changed the parameters
dictionary key to a (string, Type?) tuple
ICommonSession
parserCommandDiscriminator
equality override not actually overriding the default record struct equality. This was causing it to constantly add new entries to the_concreteImplementations
dictionary.Removes an obsolete & broken not-implemented exceptionactually it appears to still be requiredILocalizationManager
to parsers & commands.