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

[Merged by Bors] - fix({ tactic/fin_cases + test/interval_cases }): add focus1 and a test #16752

Closed
wants to merge 4 commits into from

Conversation

adomani
Copy link
Collaborator

@adomani adomani commented Oct 2, 2022

Fix this bug, reported by @hrmacbeth.

The current solution is to move the focus1 to fin_cases_at, instead of relying on fin_cases and interval_cases to call focus1. Having interval_cases wrapped in focus1 is the reason for this PR.

An older (compiling) solution was to just use focus1 inside interval_cases, but calling it in fin_cases_at seems more stable.

The link above is to a Zulip discussion with more details.


Open in Gitpod

@adomani adomani added awaiting-review The author would like community review of the PR bug t-meta Tactics, attributes or user commands labels Oct 2, 2022
@hrmacbeth hrmacbeth added the modifies-tactic-syntax This PR adds a new interactive tactic or modifies the syntax of an existing tactic. label Oct 2, 2022
@hrmacbeth
Copy link
Member

hrmacbeth commented Oct 2, 2022

Adding "modifies tactic syntax" just in case. It only modifies the behaviour of interval_cases when a goal state features more than one goal, and in mathlib that shouldn't occur.

A tactics expert should review, but LGTM.

@adomani adomani changed the title fix({ tactic/interval_cases + test }): add focus1 and a test fix({ tactic/fin_cases + test }): add focus1 and a test Oct 3, 2022
@adomani adomani changed the title fix({ tactic/fin_cases + test }): add focus1 and a test fix({ tactic/fin_cases + test/interval_cases }): add focus1 and a test Oct 3, 2022
@fpvandoorn
Copy link
Member

LGTM

bors merge

@github-actions github-actions bot added ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) and removed awaiting-review The author would like community review of the PR labels Oct 3, 2022
bors bot pushed a commit that referenced this pull request Oct 3, 2022
…est (#16752)

Fix [this bug](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/interval_cases.20bug), reported by @hrmacbeth.

The current solution is to move the `focus1` to `fin_cases_at`, instead of relying on `fin_cases` and `interval_cases` to call `focus1`.  Having `interval_cases` wrapped in `focus1` is the reason for this PR.

An older (compiling) solution was to just use `focus1` inside `interval_cases`, but calling it in `fin_cases_at` seems more stable.

The link above is to a Zulip discussion with more details.
@bors
Copy link

bors bot commented Oct 3, 2022

Pull request successfully merged into master.

Build succeeded:

@bors bors bot changed the title fix({ tactic/fin_cases + test/interval_cases }): add focus1 and a test [Merged by Bors] - fix({ tactic/fin_cases + test/interval_cases }): add focus1 and a test Oct 3, 2022
@bors bors bot closed this Oct 3, 2022
@bors bors bot deleted the adomani_focus1_ic branch October 3, 2022 12:02
@eric-wieser eric-wieser added the hacktoberfest-accepted Without this label hacktoberfest is scared off by bors label Oct 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug hacktoberfest-accepted Without this label hacktoberfest is scared off by bors modifies-tactic-syntax This PR adds a new interactive tactic or modifies the syntax of an existing tactic. ready-to-merge All that is left is for bors to build and merge this PR. (Remember you need to say `bors r+`.) t-meta Tactics, attributes or user commands
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants