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

Added multiple tokenizers implementing new ITokenizer interface #212

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Ehryk
Copy link

@Ehryk Ehryk commented Aug 1, 2015

Persuant to issue #108, I have renamed the default tokenizer to TokenizerGetOpt, and added TokenizerWindows (currently just '/', '//' variant of TokenizerGetOpt), and TokenizerHybrid (allowing both '-','/' and '--','//' together). I also added a ITokenizer interface to set a standard for possibly other tokenizers.

Because static classes cannot implement interfaces, the tokenizers are no longer static and must be instantiated before use (which is now done so using TokenizerGetOpt).

@gsscoder
Copy link
Owner

gsscoder commented Aug 1, 2015

@Ehryk, thanks for PR! I'm not agree to support an hybrid tokenizer but the other parts seem OK.

As you can see I've recently promoted the library from alpha to beta and except fixes I'll delay accepting PR until it stabilizes.

I was about opening an announcement issue on that.

I'll back on the question ASAP.

@gsscoder
Copy link
Owner

gsscoder commented Aug 1, 2015

Please see Issue #213.

@Ehryk
Copy link
Author

Ehryk commented Aug 2, 2015

Are you comfortable with the general structure (any class that implements the ITokenizer interface can be used to tokenize the command line)?

This way even if you aren't interested in supporting a Hybrid or Windows tokenizer, they could be added and supported separately? There could even be other tokenizers added for variants (of which I am not familiar, but perhaps some other use case would desire).

@gsscoder
Copy link
Owner

gsscoder commented Aug 3, 2015

@Ehryk, are you proposing to open this part of the library and let people define their own Tokenizer?

I just want to you don't need an ITokenizer interface since a Func delegate is already its signature:

Func<IEnumerable<string>, IEnumerable<OptionSpecification>, Result<IEnumerable<Token>, Error>>

As used all over the code... :)

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

Successfully merging this pull request may close these issues.

2 participants