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

fix: make sure syntax nodes always run their formatters #4631

Merged
merged 2 commits into from
Jul 3, 2024

Conversation

kmill
Copy link
Collaborator

@kmill kmill commented Jul 2, 2024

Now syntax nodes have their formatters run even if the parsers they wrap are all arity zero. This fixes an issue where if ppSpace appears in a macro/elab then it does not format with a space due to the fact that macro argument processing wraps this as group(ppSpace), and ppSpace has arity zero.

Implementation note: the fix is to make the visitArgs formatter combinator always visit the last child, even if it does not exist (in which case the visited node will be Syntax.missing). To compensate, parser combinators like many and optional need to be sure to keep track of whether there any children. Only optional's needed to be modified.

Closes #4561

This fixes an issue where if `ppSpace` appears in a `macro`/`elab` then it does not format with a space due to the fact that it gets wrapped as `group(ppSpace)` in the macro argument processing.

Closes leanprover#4561
@kmill kmill requested a review from Kha as a code owner July 2, 2024 20:16
@kmill kmill marked this pull request as draft July 2, 2024 20:22
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label Jul 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 2, 2024
@kmill kmill changed the title fix: make group parser combinator always run its formatter fix: make sure group parser combinator always run its formatter Jul 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Jul 2, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Jul 2, 2024
@kmill kmill changed the title fix: make sure group parser combinator always run its formatter fix: make sure syntax nodes always run their formatters Jul 2, 2024
@kmill kmill marked this pull request as ready for review July 2, 2024 22:05
@Kha Kha added this pull request to the merge queue Jul 3, 2024
Merged via the queue into leanprover:master with commit c2edae9 Jul 3, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ppSpace in elab/macro is ignored before constructor dot notation
2 participants