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

Cliparser with grammar #324

Closed
wants to merge 11 commits into from
Closed

Conversation

peschwa
Copy link
Member

@peschwa peschwa commented Oct 23, 2014

No description provided.

@peschwa peschwa closed this Oct 23, 2014
@peschwa
Copy link
Member Author

peschwa commented Oct 23, 2014

This was a mistake. It doesn't even work. Sorry.

@peschwa
Copy link
Member Author

peschwa commented Oct 25, 2014

It does work now, and I think I probably should've left this PR open with just the note that it didn't work yet.

I'm not sure in how far the PR is linked to the branch I want to merge into rakudo, but if this PR always tracks the branch as-is, and not as-was-when-the-PR-was-opened, then reopening should work, I think. That's why I'm going to try that now.

As for actual information re: what this PR does: I've implemented a grammar for parsing commandline options and their values as well as arguments. The grammar lives in Perl6::CommandLine::Parser and gives back a HLL::CommandLine::Result, just as HLL::CommandLine::Parser did. With a grammar we can rather easily handle things like the delimited options which S19 specs. Those also work, even though they don't go through to any specific subsystem yet, e.g.:

    $ ./perl6 ++BUG --frontend=Debugger::CommandLine ++/BUG -e'say %*OPTS'
    "BUG" => "--frontend=Debugger::CommandLine"

I suppose subsystem options could be stored in some parsed structure inside %*OPTS, but S19 is a little unclear about this matter.

Additionally, : for long options as well as negation with / works, e.g.

    perl6 :stagestats -/h -e1

prints stagestats, but doesn't print &usage.

Parsing of options with optional values (like --doc and --stagestats) is implemented to need an equals sign = when a value is passed, because we can't rely on an optional value not looking like the next option and the other way around.

@peschwa peschwa reopened this Oct 25, 2014
@vendethiel
Copy link
Contributor

(Yes, re-opening works! Github just picks it up. However, you can't rebase PRs)

This obviously means that it doesn't really work.
This obviously makes thorough testing rather difficult,
but at least stopping on -e and -- works as well as
multiple -I.
This is barely a semi-clean solution. The core problem is
--stagestats having an optional value, this only prevents
it taking values that start with '-'.
Apparently the TOP rule in Perl6::CommandLine::Parser was
bogus, and it didn't parse the case "only one argument",
which resulted in not being able to rune make spectest.
This also included a few bug fixes, spectest runs and
is clean.
@peschwa
Copy link
Member Author

peschwa commented Jan 4, 2015

Rebased onto nom as of today, to clean this PR up a little.

@moritz
Copy link
Member

moritz commented Jan 11, 2015

I'm not at all happy with the approach of joining the command line arguments with a zero-byte and then re-parsing them. Even if we can ensure that all our platforms disallow zero-bytes within the command line arguments (and I have no idea if maybe windows has an alternative API that allows it), it still feels very wrong.

Maybe this is the road to world domination, but I don't see it yet.

I'd welcome comments from other Rakudo developers.

@timo
Copy link
Member

timo commented Nov 26, 2016

i'd like to see this in rakudo. we can use a private use unicode character instead of a nullbyte to get around that particular piece of worry.

@AlexDaniel
Copy link
Contributor

private use unicode character instead of a nullbyte

I don't think it's better.

@AlexDaniel AlexDaniel changed the base branch from nom to master August 27, 2019 22:35
@JJ
Copy link
Collaborator

JJ commented May 3, 2020

This is way outdated, and in conflict with a good bit of the codebase. Also, the approach has raised a certain amount of polemic, and it does not really solve any issue. If conflicts are not solved and there's some degree of support for this I'm closing it in the next few days/weeks.

@JJ
Copy link
Collaborator

JJ commented May 5, 2020

Closed for this issue in the problem-solving repo

@JJ JJ closed this May 5, 2020
@AlexDaniel
Copy link
Contributor

AlexDaniel commented May 5, 2020

So I'm looking at this and it feels like it's just not ready yet. At first I didn't see any problem with joining things with a null byte (as indeed you cannot have null bytes in your arguments), but then I saw this:

[ '--' <.ws>+ { $*STOPPED := 1 } ]

So if you have empty arguments it'll just eat right through them.

And then:

<.ws>?

So we don't even know if we're in between the arguments or not?

We can of course fix it, but the whole idea of squishing things together only to reparse them later is indeed an anti-pattern of some sort.

Why does it have to be a grammar that parses the whole thing at once? Just don't do it, it'll be simpler and less buggy.

@AlexDaniel AlexDaniel reopened this May 5, 2020
@AlexDaniel AlexDaniel marked this pull request as draft May 5, 2020 16:19
@lizmat
Copy link
Contributor

lizmat commented Jan 12, 2021

I think this PR can be closed. Too much has changed since, looks like. If nobody objects, I will close this PR in the coming days.

@coke
Copy link
Collaborator

coke commented Oct 15, 2021

Closing based on last several comments. Thank you for the effort

@coke coke closed this Oct 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants