Skip to content
This repository has been archived by the owner on Apr 22, 2021. It is now read-only.

Adds KWARG, KWARG_LIST, and KWARG_IND_LIST to make parsing function calls easier #55

Closed
wants to merge 1 commit into from

Conversation

ajm188
Copy link
Contributor

@ajm188 ajm188 commented Sep 7, 2016

Similar to EXPR, EXPR_LIST, EXPR_IND_LIST, but actually simpler

@evhub
Copy link
Contributor

evhub commented Sep 8, 2016

@ajm188 I feel like pretty much whenever this would be useful, you would want to do EXPR | KWARG, Optional(EXPR_LIST) + Optional(KWARG_LIST), or Optional(EXPR_IND_LIST) + Optional(KWARG_IND_LIST), instead of just using KWARG, KWARG_LIST, KWARG_IND_LIST on their own. Furthermore, if you want to match Python function call parameters, you have to include *args, **kwargs as well. Additionally, since such parameters should always appear inside of parens, there's no reason not to just always use KWARGS_IND_LIST instead of KWARGS_LIST, which makes KWARGS_LIST basically useless and a trap for people who should be using KWARGS_IND_LIST. Thus, instead of KWARG, KWARG_LIST, KWARG_IND_LIST, I propose PARAM, PARAM_LIST that would look something like:

PARAM = condense(Optional(Word("*") | NAME + Literal("=")) + EXPR)
PARAM_LIST = originalTextFor(PARAM + ZeroOrMore(COMMA_IND + PARAM) + Optional(COMMA_IND))

@evhub
Copy link
Contributor

evhub commented Sep 13, 2016

@ajm188 Added PARAM, PARAMS as above. Closing.

(Apologies for committing that directly into master--I thought I was on my fork by mistake. But everything's passing, so there's no reason to revert it.)

@evhub evhub closed this Sep 13, 2016
@ajm188
Copy link
Contributor Author

ajm188 commented Sep 14, 2016

Ack, yeah I was gonna say "pull requests in the future :)" but hey it happens.

Recommend cutting a feature branch for stuff (even if you're on your fork)

@ajm188 ajm188 deleted the adds-kwarg-lists branch September 22, 2016 17:34
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants