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: simp to still work even if one simp arg does not work #4177

Merged
merged 8 commits into from
Jun 3, 2024

Conversation

nomeata
Copy link
Collaborator

@nomeata nomeata commented May 15, 2024

this fixes a usability paper cut that just annoyed me. When editing a
larger simp proof, I usually want to see the goal state after the simp,
and this is what I see while the simp command is complete. But then,
when I start typing, and necessarily type incomplete lemma names, that
error makes simp do nothing again and I see the original goal state.
In fact, if a prefix of the simp theorem name I am typing is a valid
identifier, it jumps even more around.

With this PR, using logException, I still get the red squiggly lines
for the unknown identifer, but simp just ignores that argument and
still shows me the final goal. Much nicer.

I also demoted the message for [-foo] when foo isn’t simp to a
warning and gave it the correct ref.

See it in action here: (in the middle, when you suddenly see the terminal,
I am switching lean versions.)

Peek.2024-05-15.18-38.mp4

this fixes a usability paper cut that just annoyed me. When editing a
larger simp proof, I usually want to see the goal state after the simp,
and this is what I see while the `simp` command is complete. But then,
when I start typing, and necessarily type incomplete lemma names, that
error makes `simp` do nothing again and I see the original goal state.
In fact, if a prefix of the simp theorem name I am typing is a valid
identifier, it jumps even more around.

With this PR, using `logException`, I still get the red squiggly lines
for the unknown identifer, but `simp` just ignores that argument and
still shows me the final goal. Much nicer.

I also demoted the message for `[-foo]` when `foo` isn’t `simp` to a
warning and gave it the correct `ref`.
@nomeata nomeata added the awaiting-review Waiting for someone to review the PR label May 15, 2024
@nomeata nomeata requested review from leodemoura and kim-em as code owners May 15, 2024 16:46
@nomeata nomeata added the new-user-papercuts Issue likely to confuse or otherwise negatively affect new users, even if only a little label May 15, 2024
@github-actions github-actions bot added the toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN label May 15, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

Mathlib CI status (docs):

  • ❗ Batteries/Mathlib CI will not be attempted unless your PR branches off the nightly-with-mathlib branch. Try git rebase 82666e5e7cfc541659459bd450f5f1b24a4080eb --onto 3035d2f8f689b52963f49b2414414913ca296953. (2024-05-15 17:04:36)

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 15, 2024 17:19 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 16, 2024 13:59 Inactive
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc May 16, 2024 15:48 Inactive
@david-christiansen
Copy link
Contributor

This has also bothered me - thanks for addressing it!

@nomeata
Copy link
Collaborator Author

nomeata commented May 29, 2024

Do you feel confident enough about the implementation to supplement my confidence and approve :-)

@nomeata nomeata added this pull request to the merge queue Jun 3, 2024
Merged via the queue into master with commit f65e3ae Jun 3, 2024
12 checks passed
Komyyy pushed a commit to Komyyy/lean4 that referenced this pull request Jun 4, 2024
…er#4177)

this fixes a usability paper cut that just annoyed me. When editing a
larger simp proof, I usually want to see the goal state after the simp,
and this is what I see while the `simp` command is complete. But then,
when I start typing, and necessarily type incomplete lemma names, that
error makes `simp` do nothing again and I see the original goal state.
In fact, if a prefix of the simp theorem name I am typing is a valid
identifier, it jumps even more around.

With this PR, using `logException`, I still get the red squiggly lines
for the unknown identifer, but `simp` just ignores that argument and
still shows me the final goal. Much nicer.

I also demoted the message for `[-foo]` when `foo` isn’t `simp` to a
warning and gave it the correct `ref`.

See it in action here: (in the middle, when you suddenly see the
terminal,
I am switching lean versions.)


https://github.com/leanprover/lean4/assets/148037/8cb3c563-1354-4c2d-bcee-26dfa1005ae0
kim-em added a commit to leanprover-community/mathlib4 that referenced this pull request Jun 4, 2024
nomeata added a commit that referenced this pull request Jun 5, 2024
this is an amendment to #4177, after @kmill pointed out an issue:

Users might expect that within a tactic combinator like `first`, `simp
[h]` fails if `h` does not exist. Therefore the behavior introduced in
PR #4177, which is really most useful in mormal interactive use of
`skip`, is restricted to when `recover := true`.
github-merge-queue bot pushed a commit that referenced this pull request Jun 5, 2024
this is an amendment to #4177, after @kmill pointed out an issue:

Users might expect that within a tactic combinator like `first`, `simp
[h]` fails if `h` does not exist. Therefore the behavior introduced in
PR #4177, which is really most useful in mormal interactive use of
`skip`, is restricted to when `recover := true`.
kmill added a commit to kmill/lean4 that referenced this pull request Oct 28, 2024
github-merge-queue bot pushed a commit that referenced this pull request Oct 28, 2024
Modifies `simp` to elaborate all simp arguments without disabling error
recovery. Like in #4177, simp arguments with elaboration errors are not
added to the simp set. Error recovery is still disabled when `simp` is
used in combinators such as `first`.

This enables better term info and features like tab completion when
there are elaboration errors.

Also included is a fix to the `all_goals` and `<;>` tactic combinators.
Recall that `try`/`catch` for the Tactic monad restores the state on
failure. This meant that all messages were being cleared on tactic
failure. The fix is to use `Tactic.tryCatch` instead, which doesn't
restore state.

Part of addressing #3831

Closes #4888
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting-review Waiting for someone to review the PR new-user-papercuts Issue likely to confuse or otherwise negatively affect new users, even if only a little toolchain-available A toolchain is available for this PR, at leanprover/lean4-pr-releases:pr-release-NNNN
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants