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

Refactor parseFunctionArgs and parseArgs #30

Open
jaredoconnell opened this issue Feb 1, 2024 · 1 comment
Open

Refactor parseFunctionArgs and parseArgs #30

jaredoconnell opened this issue Feb 1, 2024 · 1 comment

Comments

@jaredoconnell
Copy link
Contributor

          This would be a great place to handle the opening parenthesis, if it's present, _before_ calling `parseArgs()` (and then to handle the closing parenthesis, after it returns)...like it says in the function header comment.  😉

Originally posted by @webbnh in #23 (comment)

@webbnh
Copy link
Contributor

webbnh commented Feb 2, 2024

Unfortunately, the hyperlink above to the comment is not working. I think this one does, though. However, the comment's link to the "earlier comment" doesn't work either; try this one instead. (The reasons why are a long story, the short of which is that I've reported this to GitHub as a bug.)

In any case, here's what the second comment says [slightly edited]:

I have a suggestion for reworking this function, which I think will make it simpler and easier to understand:

  • before entering the loop, look for and remove the opening parenthesis -- it's supposed to be there, and it's an error if it's not;
  • then, if the current token is not a closing paren, enter the loop (i.e., don't enter the loop if there are no arguments in the list); inside the loop, parse the argument; then exit the loop if the next token is not a comma; inside the loop, continue to remove the comma and iterate;
  • after the loop, look for and remove the closing parenthesis -- it's supposed to be there, and it's an error if it's not.
    This saves you from having to have a loop iterator and from having to have special cases for the first iteration of the loop. The only conditional is the exit condition in the middle of the loop if you encounter a closing paren instead of a comma after an argument.

If we were writing out the detailed grammar, it would look like this:

<function_call> := <identifier> <argument_list>
<argument_list> := "(" [ <arguments> ] ")"
<arguments> := <argument> [ "," <argument_list> ]

in specific, the opening and closing parens are part of the argument_list production but not part of the arguments production, and so they should be parsed independently of the arguments. (E.g., the loop should arguably be in a separate function.)

So, to summarize, it would be better if parseFunctionArgs() managed the opening and closing parentheses and parseArgs() managed only the sequence of zero or more arguments separated by commas and left everything else to its caller.

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

No branches or pull requests

2 participants