Conversation
| /// The Symbol represented by the token (if any). | ||
| /// </summary> | ||
| internal Symbol? Symbol { get; set; } | ||
| public Symbol? Symbol { get; internal set; } |
There was a problem hiding this comment.
We should test that this is correctly reassigned when OnlyTake is used.
|
|
||
| /// <inheritdoc /> | ||
| public override string ToString() => Value; | ||
|
|
There was a problem hiding this comment.
Standard guidance (https://learn.microsoft.com/en-us/dotnet/api/system.iequatable-1?view=netstandard-2.0#notes-to-implementers) does suggest we should keep the equality operators for consistency, even if we aren't using them at the moment.
From a personal perspective, I like having the equality operators available since it makes the code simpler when dealing with nullable types, and using the equality operators helps avoid (via compilation errors) accidentally making typos that result in calling type1.Equals(type2) (calling into Type1.Equals(object), always returning false).
| using System.CommandLine.Parsing; | ||
| using Xunit; | ||
|
|
||
| namespace System.CommandLine.Tests |
There was a problem hiding this comment.
Are we leveraging file scoped namespaces?
I see at least one file was converted to use them by @jonsequitur in #2226
4 participants
While creating the API design proposal I've noticed that:
Tokenctor acceptsSymbol, but the property is internal. Some users already asked for exposing in the past (Make Token.Symbol property public #1815)Tokendoes not need to override the quality operators, I have most likely introduced them when I've made it a struct in the past