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

Allow tailing comma in arguments #623

Merged
merged 7 commits into from
Mar 9, 2025

Conversation

zhuliquan
Copy link
Contributor

allow the last argument append with comma in func expr.
make below expr can be parsed ok.

func1(
     argument1,
     argument2,
     argument3,
)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
@antonmedv
Copy link
Member

Althoug this is nice feature, I think for not is not needed in Expr. as most of the time expressions is one line.

We may return later to this and change our decision.

@antonmedv antonmedv closed this Mar 7, 2025
@zhuliquan
Copy link
Contributor Author

Althoug this is nice feature, I think for not is not needed in Expr. as most of the time expressions is one line.

We may return later to this and change our decision.

Although most of the time the expression is a single line, I still think this feature makes sense. This is considered from the perspective of syntactic consistency of expr. Because I discovered that expr parser supports ignoring the last comma on type of array or type of map, which is much like Python unlike JSON. For example, the following statements can be parsed via expr parser

[
   1,
   2,
]
{
  "1": 1,
  "2": 2,
}

So from this point of view, many users may naturally think that the last comma will be ignored by the parser. However, if the user finds that the function call is not allowed to add a comma at the end. This can be confusing.

@antonmedv
Copy link
Member

Hmm. This is true. We do allow comma in arrays & maps. Let me think on it bit more.

@antonmedv antonmedv reopened this Mar 8, 2025
@zhuliquan zhuliquan force-pushed the feature-last_arg_comma branch from 685ad2c to 2eed2d8 Compare March 9, 2025 02:43
@antonmedv
Copy link
Member

Well, let's add support for this!

@antonmedv antonmedv changed the title Feature last arg comma Allow tailing comma in arguments Mar 9, 2025
@antonmedv antonmedv merged commit 8f83203 into expr-lang:master Mar 9, 2025
16 checks passed
@zhuliquan zhuliquan deleted the feature-last_arg_comma branch March 9, 2025 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants