command-line-api icon indicating copy to clipboard operation
command-line-api copied to clipboard

Consider adding value AnnotationAccessors

Open KathleenDollard opened this issue 1 year ago • 2 comments

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 avatar Apr 26 '24 10:04 KathleenDollard

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 avatar Apr 26 '24 10:04 KalleOlaviNiemitalo

[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.

KalleOlaviNiemitalo avatar Apr 26 '24 10:04 KalleOlaviNiemitalo