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

Update fold syntax #730

Merged
merged 6 commits into from
Oct 22, 2024
Merged

Update fold syntax #730

merged 6 commits into from
Oct 22, 2024

Conversation

vincent-botbol
Copy link
Contributor

@vincent-botbol vincent-botbol commented Oct 22, 2024

This PR updates the fold syntax to what has been decided during the syntax committee, i.e., #647 (comment)

Warning: reusing the WITH_V token leads to a shift/reduce conflict in the parser which I can't grasp my head around. Following @denismerigoux suggestion, we'll merge this PR first and address the issue in a follow-up PR.

In particular, the last commit removes conflicting error messages which have to be reverted when the issue gets a resolution.

Copy link
Contributor

@denismerigoux denismerigoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but shouldn't the new keywords be updated in all the syntax highlighting grammars (https://github.com/CatalaLang/catala/tree/master/syntax_highlighting) as well as the Catala syntax sheat cheet ? I'm using this PR as an exercise to train you to add new syntactic features to Catala @vincent-botbol :)

@@ -194,7 +197,8 @@ let rec lazy_eval : decl_ctx -> Env.t -> laziness_level -> expr -> expr * Env.t
| ELit (LInt i) -> Runtime.o_eq_int_int one i
| ELit (LRat r) -> Runtime.o_eq_rat_rat (Runtime.decimal_of_integer one) r
| ELit (LMoney m) -> Runtime.o_eq_mon_mon (Runtime.money_of_units_int 1) m
| ELit (LDuration dt) -> Runtime.duration_to_years_months_days dt = (0, 0, 1)
| ELit (LDuration dt) ->
Runtime.duration_to_years_months_days dt = (0, 0, 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These look look like formatting changes, is it that @AltGr didn't use ocamlformaton this file when he merged a previous PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose so

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😇

@vincent-botbol vincent-botbol force-pushed the vbot@update-syntax branch 2 times, most recently from f3d8d4b to 5d79e34 Compare October 22, 2024 12:17
@vincent-botbol
Copy link
Contributor Author

OK, but shouldn't the new keywords be updated in all the syntax highlighting grammars (https://github.com/CatalaLang/catala/tree/master/syntax_highlighting) as well as the Catala syntax sheat cheet ? I'm using this PR as an exercise to train you to add new syntactic features to Catala @vincent-botbol :)

Did just that. As agreed, we will remove outdated/unmaintained tooling in a follow-up PR.

@vincent-botbol vincent-botbol merged commit a323940 into master Oct 22, 2024
5 checks passed
@vincent-botbol vincent-botbol deleted the vbot@update-syntax branch October 22, 2024 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants