Skip to content

Remove need for parentheses in many places #123

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

Open
wants to merge 37 commits into
base: main
Choose a base branch
from
Open

Remove need for parentheses in many places #123

wants to merge 37 commits into from

Conversation

pimotte
Copy link
Contributor

@pimotte pimotte commented Apr 21, 2025

Fixes #115

TODO:

  • Replace Notation hacks by proper solution? Document hack, because ocaml API needed is in Coq 8.18
  • Update waterproof-exercises Companion PR
  • Fix make build
  • Also make Choose a. work instead of Choose (a). Leave Choose (a). as-is.

@pimotte pimotte requested a review from jim-portegies April 21, 2025 13:03
Copy link
Contributor

@jim-portegies jim-portegies left a comment

Choose a reason for hiding this comment

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

I think this looks great! It's really a large improvement.

One question: As a test and for the looks of it, it may be nice to also remove parentheses in library files, although this is not strictly necessary. What's your view on this?

@pimotte
Copy link
Contributor Author

pimotte commented Apr 24, 2025

I think this looks great! It's really a large improvement.

One question: As a test and for the looks of it, it may be nice to also remove parentheses in library files, although this is not strictly necessary. What's your view on this?

I removed them incidentally, largely when I was touching stuff to add "as". I guess it would help to detect issues like the disjoint set ambiguity, so I'll probably remove more.

@pimotte
Copy link
Contributor Author

pimotte commented Apr 29, 2025

The reason Choose a. doesn't work is because it's expecting the continuation :=: Choose a := ....

Some options:

  • Leave Choose (a). as-is. It's not heavily used currently.
  • Reword Choose (a). as We show a works. or something similar.
  • Disable Choose (a). in favour of Choose a0 := a.

My gut feeling is towards the second option, but I might be missing some other option to resolve the parsing ambiguity. @jim-portegies any thoughts?

@jim-portegies
Copy link
Contributor

I'm leaning towards option 3. I don't mind 1 either. I am not the biggest fan of 2 because in principle I would like to communicate one way of doing things.

@pimotte
Copy link
Contributor Author

pimotte commented Apr 29, 2025

Fair enough. From an engineering point of view, I'd like to then go with option 1 for this PR, we can then follow up with option 3 if we feel it's worth it.

@pimotte
Copy link
Contributor Author

pimotte commented Apr 29, 2025

With some refactoring we might also be able to do Choose a. (currently, the way the parsing is structured is that the ident := is optional, where instead we might want the := open_lconstr part to be optional.

Edit: After experimenting a bit, I don't think we can do this without disabling Choose _, so in any case I'm delegating investigation to a possible future PR.

@pimotte pimotte marked this pull request as ready for review April 29, 2025 19:37
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.

Reduce number of unnecessary parentheses
3 participants