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

Incorrect parse of arguments after a redirect #233

Closed
danfuzz opened this issue Oct 31, 2023 · 2 comments · Fixed by #245
Closed

Incorrect parse of arguments after a redirect #233

danfuzz opened this issue Oct 31, 2023 · 2 comments · Fixed by #245

Comments

@danfuzz
Copy link
Contributor

danfuzz commented Oct 31, 2023

[This issue is split off from issue #232.]

Parsing the following command results in spurious redirect contents, which I think suggests that the distinction between command and redirected_statement nodes might want to be re-evaluated.

x y <x a b c

Parse tree:

(program [0, 0] - [1, 0]
  (redirected_statement [0, 0] - [0, 12]
    body: (command [0, 0] - [0, 3]
      name: (command_name [0, 0] - [0, 1]
        (word [0, 0] - [0, 1]))
      argument: (word [0, 2] - [0, 3]))
    redirect: (file_redirect [0, 4] - [0, 12]
      destination: (word [0, 5] - [0, 6])
      destination: (word [0, 7] - [0, 8])
      destination: (word [0, 9] - [0, 10])
      destination: (word [0, 11] - [0, 12]))))

I think the direct cause of this mis-parse is the repeat1(...) in the file_redirect rule, which causes it to consume more than just one word as the destination of a redirection. (This is around line 505 of grammar.js.)

The parse should really have more argument fields after the redirect, but given the existing structure those arguments would have to be directly under redirected_statement, not body. This is why I commented above about the distinction between redirected_statement and command.

@danfuzz
Copy link
Contributor Author

danfuzz commented Nov 1, 2023

Not 100% sure, but I think I'd suggest dropping redirected_statement entirely, and then making command be defined as (something like) a sequence of repeat(redirect:redirect) name:literal repeat(argument:literal | redirect:redirect).

@amaanq amaanq mentioned this issue Feb 10, 2024
@ObserverOfTime
Copy link
Member

The issue is still present.

printf > "$FOO" 'bar'
(program [0, 0] - [1, 0]
  (redirected_statement [0, 0] - [0, 21]
    body: (command [0, 0] - [0, 6]
      name: (command_name [0, 0] - [0, 6]
        (word [0, 0] - [0, 6])))
    redirect: (file_redirect [0, 7] - [0, 21]
      destination: (string [0, 9] - [0, 15]
        (simple_expansion [0, 10] - [0, 14]
          (variable_name [0, 11] - [0, 14])))
      destination: (raw_string [0, 16] - [0, 21]))))

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 a pull request may close this issue.

2 participants