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

Excessive injection of Suppress in infix_notation() #375

Closed
pprados opened this issue Mar 22, 2022 · 2 comments · Fixed by #376
Closed

Excessive injection of Suppress in infix_notation() #375

pprados opened this issue Mar 22, 2022 · 2 comments · Fixed by #376

Comments

@pprados
Copy link
Contributor

pprados commented Mar 22, 2022

If I want to use delimited_list() WITHOUT suppression of lpar and rpar.

The default value use Suppress("(")

def infix_notation(
    base_expr: ParserElement,
    op_list: List[InfixNotationOperatorSpec],
    lpar: Union[str, ParserElement] = Suppress("("),
    rpar: Union[str, ParserElement] = Suppress(")"),
) -> ParserElement:

but later in the code, the Suppress() is injected another time.

// Line 808
    lpar = Suppress(lpar)
    rpar = Suppress(rpar)

I would like to invoke infix_notation(...,lpar="(", rpar=")") to remove the suppression, but it's impossible.

I propose to remove the lines [808-809] to fix this bug.

Why I want this?
I want to convert a source code to another source code, but after applying pyparsing. I the middle, I can inject some "macro".

@ptmcg
Copy link
Member

ptmcg commented Mar 22, 2022

Good suggestion, we definitely don't want to double suppress lpar and rpar. But I do want to auto-promote str's to Suppress, which is pretty common throughout pyparsing's API (where a str is passed in and is auto-promoted to Suppress or Literal).

Can you submit a PR where the lines are written as:

if isinstance(lpar, str):
    lpar = Suppress(lpar)
if isinstance(rpar, str):
    rpar = Suppress(rpar)

You could then invoke infix_notation as infix_notation(..., lpar=Literal("("), rpar=Literal(")")), and the paren characters would be retained, and not suppressed.

@pprados pprados mentioned this issue Mar 24, 2022
ptmcg pushed a commit that referenced this issue Mar 24, 2022
@ptmcg
Copy link
Member

ptmcg commented Mar 24, 2022

After adding unit tests for this change, it looks odd that the grouping symbols are not grouped with their contents. I've modified the code to add them, plus notes in the HowToUsePyparsing.rst doc. See https://github.com/pyparsing/pyparsing/blob/master/docs/HowToUsePyparsing.rst#31helper-methods

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