Skip to content

Lacking string -> enumerable<string> #41

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

Closed
ericnewton76 opened this issue Nov 4, 2017 · 6 comments
Closed

Lacking string -> enumerable<string> #41

ericnewton76 opened this issue Nov 4, 2017 · 6 comments

Comments

@ericnewton76
Copy link
Member

Issue by davhdavh
Friday Aug 28, 2015 at 10:13 GMT
Originally opened as gsscoder/commandline#232


There seems to be no support for NOT depending on the OS parsing of the arguments string.
In my case, I am parsing a command line from a chat program.

@ericnewton76
Copy link
Member Author

Comment by gsscoder
Friday Aug 28, 2015 at 20:04 GMT


@davhdavh,
this is true. Simply splitting by whitespace is not the same than the actual work performed by a terminal.

I think that terminal call some platform specific API.

This will be a cool enhancement and will be scheduled when it reaches stable status.

I've already reasoned of it in past, but nobody asked for it. I think too it could be useful! :)

Thanks for posting it! (Please be patient until -> stable). 👍

@ericnewton76
Copy link
Member Author

Comment by gsscoder
Tuesday Sep 01, 2015 at 03:05 GMT


Command Line actually include some source file with Paket.

These files are not included in the git repo, but you can get it from a terminal:

$ cd CommandLine
$ .\packages\Paket.1.19.7\tools install

As said in first post, I can't accept PR of type enhancement until beta becomes stable (and fit in this state without serious issues for a some time, to be fair).

I've not done a detailed code review, but in general keep in mind that 2.0.x was born for rewrite the kernel in functional style.

Your pre-tokenizer could be in static class with a static member: PreTokenizer.Tokenize(string) : IEnumerable<string>.

Could be public or we can add a ParseArguments overload that accepts a string instead of a string[].

But please wait for stable... Anyway thanks for your ideas and work. :)

@ericnewton76
Copy link
Member Author

Comment by davhdavh
Tuesday Sep 01, 2015 at 04:27 GMT


That's fine, you have the code for the actual parsing when you need it.
Plus tests for them.
You can find a linux-type parser at
http://sourcecodebrowser.com/glib2.0/2.25.7/gshell_8c_source.html for
inspiration (it is vastly more complex than the two I made)

On Tue, Sep 1, 2015 at 10:05 AM, Giacomo Stelluti Scala <
notifications@github.com> wrote:

Command Line actually include some source file with Paket.

These files are not included in the git repo, but you can get it from a
terminal:

$ cd CommandLine
$ .\packages\Paket.1.19.7\tools install

As said in first post, I can't accept PR of type enhancement until beta
becomes stable (and fit in this state without serious issues for a some
time, to be fair).

I've not done a detailed code review, but in general keep in mind that
2.0.x was born for rewrite the kernel in functional style.

Your pre-tokenizer could be in static class with a static member: PreTokenizer.Tokenize(string)
: IEnumerable.

Could be public or we can add a ParseArguments overload that accepts a
string instead of a string[].

But please wait for stable... Anyway thanks for your ideas and work. :)


Reply to this email directly or view it on GitHub
gsscoder/commandline#232 (comment)
.

Dennis

@ericnewton76
Copy link
Member Author

Comment by gsscoder
Saturday Sep 05, 2015 at 15:20 GMT


@davhdavh, thanks for great discussion and contribution :)

will re-start it after stable is released.

👍

@ericnewton76
Copy link
Member Author

Comment by Mizipzor
Thursday Jan 28, 2016 at 13:03 GMT


Does the 2.0.x-stable label mean that this issue has to be solved for the 2.0 branch to become stable, or that the issue wont be worked on until 2.0 is stable?

@ericnewton76
Copy link
Member Author

Comment by nemec
Saturday Jan 30, 2016 at 03:08 GMT


Sounds like this is intended to be worked on after stable is reached.
We had a discussion on the string->string[] parsing in #270. Using CommandLineToArgvW directly would limit the library to Windows though. Our best best to maintain backwards compatibility would be to write a parsing algorithm (in C#) that matches the behavior of CommandLineToArgvW as closely as possible and add an additional overload to the Parse method that takes a single string.

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

No branches or pull requests

2 participants