[Merged by Bors] - chore: remove some stream-of-conciousness syntax for obtain#12850
Closed
[Merged by Bors] - chore: remove some stream-of-conciousness syntax for obtain#12850
Conversation
|
🚀 Pull request has been placed on the maintainer queue by adomani. |
eric-wieser
approved these changes
May 12, 2024
Member
eric-wieser
left a comment
There was a problem hiding this comment.
bors merge
Ouch, not just stream-of-consciousness style, but without subgoal scoping too!
Contributor
|
bors merge |
Contributor
|
Already running a review |
Contributor
|
Pull request successfully merged into master. Build succeeded: |
callesonne
pushed a commit
that referenced
this pull request
May 16, 2024
grunweg
added a commit
that referenced
this pull request
May 17, 2024
This was referenced May 25, 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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Similar to #11045; discovered in #12834.