Skip to content

[Merged by Bors] - chore: remove some stream-of-conciousness syntax for obtain#12850

Closed
grunweg wants to merge 1 commit intomasterfrom
MR-more-obtain
Closed

[Merged by Bors] - chore: remove some stream-of-conciousness syntax for obtain#12850
grunweg wants to merge 1 commit intomasterfrom
MR-more-obtain

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented May 12, 2024

Similar to #11045; discovered in #12834.


Open in Gitpod

@grunweg grunweg added awaiting-review easy < 20s of review time. See the lifecycle page for guidelines. labels May 12, 2024
Copy link
Copy Markdown
Contributor

@adomani adomani left a comment

Choose a reason for hiding this comment

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

maintainer merge

@github-actions
Copy link
Copy Markdown

🚀 Pull request has been placed on the maintainer queue by adomani.

@github-actions github-actions bot added the maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. label May 12, 2024
Copy link
Copy Markdown
Member

@eric-wieser eric-wieser left a comment

Choose a reason for hiding this comment

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

bors merge

Ouch, not just stream-of-consciousness style, but without subgoal scoping too!

@ghost ghost added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels May 12, 2024
mathlib-bors bot pushed a commit that referenced this pull request May 12, 2024
@kim-em
Copy link
Copy Markdown
Contributor

kim-em commented May 12, 2024

bors merge

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented May 12, 2024

Already running a review

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented May 12, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: remove some stream-of-conciousness syntax for obtain [Merged by Bors] - chore: remove some stream-of-conciousness syntax for obtain May 12, 2024
@mathlib-bors mathlib-bors bot closed this May 12, 2024
@mathlib-bors mathlib-bors bot deleted the MR-more-obtain branch May 12, 2024 23:39
callesonne pushed a commit that referenced this pull request May 16, 2024
grunweg added a commit that referenced this pull request May 17, 2024
mathlib-bors bot pushed a commit that referenced this pull request May 26, 2024
Concludes the work started in (TTBOMK) #11045 and #12850.
To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately.

This uncovered some interesting interactions with the multi-goal linter:
in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
callesonne pushed a commit that referenced this pull request Jun 4, 2024
Concludes the work started in (TTBOMK) #11045 and #12850.
To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately.

This uncovered some interesting interactions with the multi-goal linter:
in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
js2357 pushed a commit that referenced this pull request Jun 18, 2024
Concludes the work started in (TTBOMK) #11045 and #12850.
To make sure this is exhaustive, I wrote a syntax linter for this, which I will propose separately.

This uncovered some interesting interactions with the multi-goal linter:
in a few instances, about using the newer syntax, the sub-goal focusing also became obsolete.
mathlib-bors bot pushed a commit that referenced this pull request Jun 24, 2024
Just the syntax replacements are split into #13219.

Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances.

**Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534:
- it's a Lean3-ism, which can be unlearned now
- the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact`
- when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot
(This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) 
- the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer.

Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all,
but moves forward with something that is doable within mathlib today.
kbuzzard pushed a commit that referenced this pull request Jun 26, 2024
Just the syntax replacements are split into #13219.

Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances.

**Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534:
- it's a Lean3-ism, which can be unlearned now
- the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact`
- when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot
(This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) 
- the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer.

Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all,
but moves forward with something that is doable within mathlib today.
dagurtomas pushed a commit that referenced this pull request Jul 2, 2024
Just the syntax replacements are split into #13219.

Most of its occurrences were removed in #11045 and #12850; #13219 removed the last instances.

**Why is this removed?** In a nutshell, my understanding is that this is preferred against for similar reasons as #10534:
- it's a Lean3-ism, which can be unlearned now
- the syntax `obtain foo : type := proof` is slightly shorter; particularly so when the next tactic is `exact`
- when using it as `obtain foo : type; · proof`, there is an intermediate state with multiple goals right before the focusing dot
(This gets amplified with the in-flight "multiple goal linter", which in my perception seems generally desired - for many reasons, including teachability. Granted, the linter could be tweaked to not lint in this case... but by now, the "old" syntax is not clearly better.) 
- the old syntax *could* be slightly nicer when deferring goals: In the 30 replacements of the last PR, this occurred *twice* (i.e., very rarely). In both cases, `suffices` or `rsuffices` can be used, and would imho be clearer.

Alternatively, the `obtain` tactic in Lean core could also be changed: this change does not block this at all,
but moves forward with something that is doable within mathlib today.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

easy < 20s of review time. See the lifecycle page for guidelines. maintainer-merge A reviewer has approved the changed; awaiting maintainer approval. ready-to-merge This PR has been sent to bors.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants