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

Make Token.Symbol property public #1815

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

kahlkevin
Copy link

Motivation:

  • Inside custom middleware, it may be desirable to understand relative order of symbols in the parsed command line (for e.g. in the case of several option types that influence a shared setting and that have a "last option specified wins" semantic).
  • Best way to do understand sequence order is by inspecting the ParseResult.Tokens list. This gives Tokens, which contain embedded symbol references.
  • From there, we'd like to compare the token's Symbol directly against predefined Argument and Option objects to ease identification.
  • Doing this from non-internal code today requires something like if (tokenOfInterest == new Token(tokenOfInterest.Value, TokenType.Option, optionOfInterest)) { /* match */ ... }, which works because of the overridden Token.Equals method.
  • It'd be much easier to say if (tokenOfInterest.Symbol == optionOfInterest) { /* match */ ... }
  • No clear reason why Token.Symbol needs to be internal

Motivation:
 - Inside custom middleware, it may be desirable to understand relative order of symbols in the parsed command line (for e.g. in the case of several option types that influence a shared setting and that have a "last option specified wins" semantic).
 - Best way to do understand sequence order is by inspecting the `ParseResult.Tokens` list. This gives `Token`s, which contain embedded symbol references.
 - From there, we'd like to compare the token's `Symbol` directly against predefined `Argument` and `Option` objects to ease identification.
 - Doing this from non-`internal` code today requires something like `if (tokenOfInterest == new Token(tokenOfInterest.Value, TokenType.Option, optionOfInterest)) { /* match */ ... }`, which works because of the overridden `Token.Equals` method.
 - It'd be much easier to say `if (tokenOfInterest.Symbol == optionOfInterest) { /* match */ ... }`
 - No clear reason why `Token.Symbol` needs to be `internal`
@KalleOlaviNiemitalo
Copy link

This might simplify the redaction that I do in #1795.

@jonsequitur jonsequitur added the enhancement New feature or request label Aug 31, 2022
@jonsequitur
Copy link
Contributor

While this might be a useful API addition, it was added primarily for internal bookkeeping and it would need to be tested more thoroughly for a number of scenarios including cases where the token is shifted during parsing, i.e. ArgumentResult.OnlyTake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants