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

feat: upstream addHaveSuggestion and addRewriteSuggestion #210

Merged
merged 14 commits into from
Jan 27, 2024

Conversation

kim-em
Copy link
Collaborator

@kim-em kim-em commented Aug 8, 2023

Upstreams two more "Try this:" builders.

@kim-em kim-em added the awaiting-review This PR is ready for review; the author thinks it is ready to be merged. label Aug 8, 2023
@kim-em kim-em force-pushed the addRewriteSuggestion branch from ed72df0 to db96f32 Compare August 10, 2023 00:14
@kim-em kim-em requested a review from digama0 August 10, 2023 00:16
@kim-em
Copy link
Collaborator Author

kim-em commented Aug 25, 2023

WIP

@github-actions github-actions bot added WIP work in progress and removed awaiting-review This PR is ready for review; the author thinks it is ready to be merged. labels Aug 25, 2023
@kim-em
Copy link
Collaborator Author

kim-em commented Aug 25, 2023

awaiting-review

@github-actions github-actions bot added awaiting-review This PR is ready for review; the author thinks it is ready to be merged. and removed WIP work in progress labels Aug 25, 2023
Std/Tactic/TryThis.lean Outdated Show resolved Hide resolved
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Oct 24, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Oct 24, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Dec 13, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Dec 14, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Dec 21, 2023
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot removed the merge-conflict This PR has merge conflicts with the `main` branch which must be resolved by the author. label Jan 5, 2024
if prop then
`(tactic| have : $tstx := $estx)
else
`(tactic| let this : $tstx := $estx)
Copy link
Member

Choose a reason for hiding this comment

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

does this put this† in the output?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, it does, and there's a FIXME in Mathlib's test/propose.lean about this.

Copy link
Member

Choose a reason for hiding this comment

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

it should use $(mkIdent `this) instead

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I've fixed the behaviour of Mathlib's have? and observe? in leanprover-community/mathlib4#9454, and will modify this PR accordingly.

Copy link
Contributor

@joehendrix joehendrix left a comment

Choose a reason for hiding this comment

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

I don't have context of how these are used in Mathlib, but the operations themselves seem useful.

@joehendrix joehendrix added will-merge-soon PR will be merged soon. Any concerns should be raised quickly. and removed awaiting-review This PR is ready for review; the author thinks it is ready to be merged. labels Jan 25, 2024
@digama0 digama0 merged commit 6f352d4 into leanprover-community:main Jan 27, 2024
1 check passed
fgdorais pushed a commit to fgdorais/batteries that referenced this pull request Feb 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
will-merge-soon PR will be merged soon. Any concerns should be raised quickly.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants