Skip to content

[Merged by Bors] - chore: remove remaining stream-of-conciousness obtain syntax#13219

Closed
grunweg wants to merge 8 commits intomasterfrom
MR-remove-obtain-syntax
Closed

[Merged by Bors] - chore: remove remaining stream-of-conciousness obtain syntax#13219
grunweg wants to merge 8 commits intomasterfrom
MR-remove-obtain-syntax

Conversation

@grunweg
Copy link
Copy Markdown
Contributor

@grunweg grunweg commented May 25, 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.


Open in Gitpod

@grunweg grunweg added awaiting-review tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip labels May 25, 2024
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented May 26, 2024

I have addressed all comments.

@urkud
Copy link
Copy Markdown
Member

urkud commented May 26, 2024

Thanks! 🎉
bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels May 26, 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.
@grunweg
Copy link
Copy Markdown
Contributor Author

grunweg commented May 26, 2024

Thank you for the review!

@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented May 26, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title chore: remove remaining stream-of-conciousness obtain syntax [Merged by Bors] - chore: remove remaining stream-of-conciousness obtain syntax May 26, 2024
@mathlib-bors mathlib-bors bot closed this May 26, 2024
@mathlib-bors mathlib-bors bot deleted the MR-remove-obtain-syntax branch May 26, 2024 23:06
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

ready-to-merge This PR has been sent to bors. tech debt Tracking cross-cutting technical debt, see e.g. the "Technical debt counters" stream on zulip

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants