Skip to content

Conversation

@smondet
Copy link
Contributor

@smondet smondet commented Mar 7, 2025

Cf. #391

I tried adding to the menhir grammar but would get shift/reduce conflicts and/or would be incomplete.

Catching and improving the error message is what Menhir themselves do:
https://gitlab.inria.fr/fpottier/menhir/blob/master/src/stage2/Driver.ml

PR checklist

  • New code has tests to catch future regressions
  • Documentation is up-to-date
  • CHANGES.md is up-to-date

@smondet smondet force-pushed the sebmondet-parser-generic-error-i391 branch from 02b7773 to c7e1928 Compare March 7, 2025 17:47
@smondet smondet force-pushed the sebmondet-parser-generic-error-i391 branch from c7e1928 to 79afcab Compare March 7, 2025 17:52
@smondet
Copy link
Contributor Author

smondet commented Mar 7, 2025

 $ cat > /tmp/bug.atd <<EOF
type t = {
  type: string;
  world: float;
}
EOF
 $ dune exec atdgen/bin/ag_main.exe -- /tmp/bug.atd
File "/tmp/bug.atd", line 3, characters 2-6: Syntax error

When Menhir does the job itself it looks slightly different:

 $ cat > /tmp/bug.atd <<EOF
type x = int
type t = {
  different: case;
  type: string;
  world: float;
}
EOF
 $ dune exec atdgen/bin/ag_main.exe -- /tmp/bug.atd
File "/tmp/bug.atd", line 4, characters 2-6:
Expecting '}'

(type is not the first field here)

@smondet smondet marked this pull request as ready for review March 7, 2025 17:54
@smondet smondet requested a review from mjambon as a code owner March 7, 2025 17:55
Copy link
Collaborator

@mjambon mjambon left a comment

Choose a reason for hiding this comment

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

Nice!

@mjambon
Copy link
Collaborator

mjambon commented Mar 8, 2025

Note that if you write Closes #391 on its own line in the PR description, GitHub will close the issue for you when this gets merged.

@smondet smondet merged commit f795655 into master Mar 10, 2025
2 of 3 checks passed
@smondet smondet mentioned this pull request Mar 10, 2025
mjambon added a commit to mjambon/opam-repository that referenced this pull request Dec 10, 2025
CHANGES:

* atdgen: Add option `-j-gen-modules` to generate JSON generic submodules (ahrefs/atd#420)
* atd-parser: improve (syntax) error messages (ahrefs/atd#426)
* atdts: Add support for `<ts from...>` annotations
* atdpy: Add support for `<doc text=...>` annotations, turning them
  into docstrings similar to the ocamldoc comments produced by atdgen.
* atdgen: Remove option `-j-std`, now it's the default, one cannot
  generate extended-JSON (ahrefs/atd#425). Options `-j-std` and `-std-json` are
  still available as backward-compatibility no-ops unless
  environment variable `ATDGEN_FAIL_DEPRECATED_OPTIONS` is set to `true` in
  which case their use results in an exception.
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.

3 participants