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

Loop argument parsing issue #249

Merged
merged 5 commits into from
May 11, 2021
Merged

Loop argument parsing issue #249

merged 5 commits into from
May 11, 2021

Conversation

angelhof
Copy link
Member

@angelhof angelhof commented May 7, 2021

The loop1.sh testcase (simplified from a POSIX test) is parsed wrongly by pash which only parses the first loop argument A and not B.

One can reproduce this by running $PASH_TOP/pa.sh -d 1 loop1.sh with the added logs in this branch to see that it is a parsing issue.

@thurstond, @mgree do you have any ideas what could be the problem?

@thurstond
Copy link
Collaborator

I've just committed 2476ec0, which changes the for loop parsing (and unparsing) from "arg" to "args".

Note that this necessarily breaks the API i.e., for parsed loop1.sh, notice the arguments has an extra layer of nesting:

["For", [1, [[["C", 65]], [["C", 66]]], ["Command", [2, [], [[["C", 101], ["C", 99], ["C", 104], ["C", 111]], [["V", ["Normal", false, "idFor1", []]]]], []]], "idFor1"]]

@mgree If this change is correct, your upstream OCaml code needs a similar tweak.

@thurstond
Copy link
Collaborator

My branch is slightly out of sync so I will merge the braces change back in.

@mgree
Copy link
Collaborator

mgree commented May 7, 2021

Good catch! It's not an issue for Smoosh because the actual AST shim does the right thing, but it's definitely wrong in the libdash AST. Thanks! I'll make an issue for libdash.

@angelhof
Copy link
Member Author

angelhof commented May 7, 2021

Thanks everyone!

@angelhof angelhof marked this pull request as ready for review May 7, 2021 16:14
@angelhof angelhof requested a review from nvasilakis May 7, 2021 16:15
@angelhof angelhof changed the title [DRAFT] Loop argument parsing issue Loop argument parsing issue May 7, 2021
@angelhof angelhof merged commit 8b144df into main May 11, 2021
@angelhof angelhof deleted the parse-loop-fail branch July 28, 2021 09:49
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.

4 participants