Skip to content

fix: enforce set notation per-element indentation - #14350

Merged
Kha merged 2 commits into
masterfrom
push-zskuxkpuwonq
Aug 25, 2026
Merged

Kha merged 2 commits into
masterfrom
push-zskuxkpuwonq

Conversation

@Kha

@Kha Kha commented Jul 10, 2026

Copy link
Copy Markdown
Member

This PR enforces that elements of singleton notation need to indent e.g. nested application arguments, as otherwise the overlap with structure notation is simply too confusing for both humans and the upcoming formatter (would require blocking on elaboration).

@github-actions github-actions Bot added toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN mathlib4-nightly-available A branch for this PR exists at leanprover-community/mathlib4-nightly-testing:lean-pr-testing-NNNN labels Jul 10, 2026
@leanprover-bot leanprover-bot added the builds-manual CI has verified that the Lean Language Reference builds against this PR label Jul 10, 2026
@leanprover-bot

leanprover-bot commented Jul 10, 2026

Copy link
Copy Markdown
Collaborator

Reference manual CI status:

  • ✅ Reference manual branch lean-pr-testing-14350 has successfully built against this PR. (2026-07-10 12:28:03) View Log
  • 🟡 Reference manual branch lean-pr-testing-14350 build against this PR didn't complete normally. (2026-07-10 12:30:29) View Log
  • ✅ Reference manual branch lean-pr-testing-14350 has successfully built against this PR. (2026-07-10 14:23:33) View Log
  • 🟡 Reference manual branch lean-pr-testing-14350 build against this PR didn't complete normally. (2026-07-10 14:25:28) View Log
  • ❗ Reference manual CI can not be attempted yet, as the nightly-testing-2026-08-22 tag does not exist there yet. We will retry when you push more commits. If you rebase your branch onto nightly-with-manual, reference manual CI should run now. You can force reference manual CI using the force-manual-ci label. (2026-08-25 09:37:41)

@mathlib-lean-pr-testing mathlib-lean-pr-testing Bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Jul 10, 2026
@mathlib-lean-pr-testing

mathlib-lean-pr-testing Bot commented Jul 10, 2026

Copy link
Copy Markdown

Mathlib CI status (docs):

Kha added a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Jul 10, 2026
@Kha
Kha marked this pull request as ready for review July 10, 2026 13:47
@Kha Kha added the changelog-language Language features and metaprograms label Jul 10, 2026
@mathlib-lean-pr-testing mathlib-lean-pr-testing Bot added builds-mathlib CI has verified that Mathlib builds against this PR and removed breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan labels Jul 10, 2026
mathlib-nightly-testing Bot pushed a commit to leanprover-community/batteries that referenced this pull request Jul 10, 2026
mathlib-nightly-testing Bot pushed a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Jul 10, 2026
leanprover-bot added a commit to leanprover/reference-manual that referenced this pull request Jul 10, 2026

@MohammedAlkindi MohammedAlkindi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This change makes sense to me; I left one small test-coverage question inline.

otherwise could be either a two-field structure or a singleton set, which seems way too confusing
even when the elaborator can disambiguate the two.
-/
syntax "{" withPosition(term),+ "}" : term

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Could we add a small regression test for the ambiguous multiline case from the comment? Since this changes user-visible parser/indentation behavior, a test that the { aaa\n bbb } shape is rejected or parsed the intended way would help lock this down for the formatter work. There are related strict-indent tests under tests/elab_fail/byStrictIndent.lean, though any parser test location you prefer is fine.

@Kha
Kha force-pushed the push-zskuxkpuwonq branch from 41e4213 to 1ecc058 Compare August 25, 2026 09:19
@Kha Kha added the awaiting-mathlib We should not merge this until we have a successful Mathlib build label Aug 25, 2026
@Kha
Kha enabled auto-merge August 25, 2026 09:19
@Kha
Kha added this pull request to the merge queue Aug 25, 2026
mathlib-nightly-testing Bot pushed a commit to leanprover-community/batteries that referenced this pull request Aug 25, 2026
mathlib-nightly-testing Bot pushed a commit to leanprover-community/mathlib4-nightly-testing that referenced this pull request Aug 25, 2026
Merged via the queue into master with commit 761d5b7 Aug 25, 2026
28 checks passed
@mathlib-lean-pr-testing mathlib-lean-pr-testing Bot removed the awaiting-mathlib We should not merge this until we have a successful Mathlib build label Aug 25, 2026
@mathlib-lean-pr-testing

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

builds-manual CI has verified that the Lean Language Reference builds against this PR builds-mathlib CI has verified that Mathlib builds against this PR changelog-language Language features and metaprograms mathlib4-nightly-available A branch for this PR exists at leanprover-community/mathlib4-nightly-testing:lean-pr-testing-NNNN 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.

4 participants