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

Feature parenthesis node #620

Closed

Conversation

zhuliquan
Copy link
Contributor

I found there are many operator of adding parentheses in ast/print.go
for example:

expr/ast/print.go

Lines 54 to 56 in 7772ea0

if _, ok := n.Node.(*BinaryNode); ok {
return fmt.Sprintf("%s(%s)", op, n.Node.String())
}

expr/ast/print.go

Lines 91 to 101 in 7772ea0

if lwrap {
lhs = fmt.Sprintf("(%s)", n.Left.String())
} else {
lhs = n.Left.String()
}
if rwrap {
rhs = fmt.Sprintf("(%s)", n.Right.String())
} else {
rhs = n.Right.String()
}

why not add a specific type ast node for following benefits:
1、make printing string more like the expr inputted by user, i.e. resolve issue: #613.
2、Make the node's responsibilities more single.

@zhuliquan zhuliquan force-pushed the feature-parenthesis-node branch from d54f223 to 5521303 Compare April 6, 2024 16:36
@antonmedv
Copy link
Member

This is a interesting approach, unfortunately it makes other things more complex. Instead we should just feap thenary nodes in parenthesis if inside binary.

@antonmedv antonmedv closed this Apr 6, 2024
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.

2 participants