Skip to content
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

Consider adding value AnnotationAccessors #2403

Open
KathleenDollard opened this issue Apr 26, 2024 · 2 comments
Open

Consider adding value AnnotationAccessors #2403

KathleenDollard opened this issue Apr 26, 2024 · 2 comments
Labels
Powderhouse Work to isolate parser and features

Comments

@KathleenDollard
Copy link
Contributor

Value annotation accessors would do two things:

  • Be allowed only on options and arguments, clarifying their use
  • Would require that the passed value match the generic of the option or argument, making using invalid default values, converters, validators, etc. not possible

And example is default values which are stored as object?

While it would be nice to store the specific value, rather than the general one, I do not see a way that is possible. However, I think the code below would solve the two problems above:

public struct ValueAnnotationAccessor<TValue>(CliSubsystem owner, AnnotationId<TValue> id)
{
    /// <summary>
    /// The ID of the annotation
    /// </summary>
    public AnnotationId<TValue> Id { get; }
    public readonly void Set<TSymbolValue>(CliOption<TSymbolValue> symbol, TSymbolValue value)
        where TSymbolValue : TValue
        => owner.SetAnnotation(symbol, id, value);
    public readonly void Set<TSymbolValue>(CliArgument<TSymbolValue> symbol, TSymbolValue value)
        where TSymbolValue : TValue
        => owner.SetAnnotation(symbol, id, value); 
    public readonly bool TryGet(CliSymbol symbol, [NotNullWhen(true)] out TValue? value) => owner.TryGetAnnotation(symbol, id, out value);
}

If we wish this to be polymorphic with other AnnotationAccessors, it could derive from it, in which case it could also drop SetAnnotation

@KathleenDollard KathleenDollard added the Powderhouse Work to isolate parser and features label Apr 26, 2024
@KalleOlaviNiemitalo
Copy link

AnnotationId<TValue> looks similar to HttpRequestOptionsKey<TValue>.

And now I realize it has already been implemented and just doesn't show up in GitHub code search because that doesn't cover non-default branches.

@KalleOlaviNiemitalo
Copy link

[NotNullWhen(true)] out TValue? value seems strange. What if I have a nullable type (e.g. string?) as TValue and store null, and then query with TryGet; does that assign value = null and return true, thus claiming that value is not null after all? I imagine this should instead be [MaybeNullWhen(false)] out TValue value, like in Dictionary<TKey, TValue>.TryGetValue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Powderhouse Work to isolate parser and features
Projects
None yet
Development

No branches or pull requests

2 participants