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

chore: remove @ from rw? suggestions, and enable hover on constants in #check #3911

Merged
merged 2 commits into from
Apr 19, 2024

Conversation

kim-em
Copy link
Collaborator

@kim-em kim-em commented Apr 15, 2024

  • Replaces the unused Lean.PrettyPrinter.ppConst with
    MessageData.ofConst (which similarly avoids an unnecessary @) and that further generates a hover for the constant

  • Uses this in TryThis.addRewriteSuggestion, so that rw? suggestions don't have unnecessary @s.

  • Add MessageData.signature, as a wrapper around PrettyPrinter.signature, using the same machinery to generate hovers for constants, improving the hover behaviour in #check so that we get second order pop-up for constants in the signature. (Not sure how to write tests for second order hovers, so there is no test for this.)

@kim-em kim-em requested a review from digama0 as a code owner April 15, 2024 09:03
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 15, 2024 09:12 Inactive
@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 Apr 15, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 15, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 15, 2024
@leanprover-community-mathlib4-bot leanprover-community-mathlib4-bot added the breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan label Apr 15, 2024
@leanprover-community-mathlib4-bot
Copy link
Collaborator

leanprover-community-mathlib4-bot commented Apr 15, 2024

Mathlib CI status (docs):

@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 15, 2024 10:27 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 15, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 15, 2024
@kim-em
Copy link
Collaborator Author

kim-em commented Apr 15, 2024

There will be some downstream breakage, but should be minor so I'm not going to fix ahead of time.

Copy link
Collaborator

@kmill kmill left a comment

Choose a reason for hiding this comment

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

In the PR message:

improving the hover behaviour in #check.

Could you mention what's being improved in the PR message? I seem to be able to hover over everything except for the constant itself, so I'm guessing it's that this lets you hover over the constant? That makes some sense, for being able to see the docstring, even if you don't need to see the type.

src/Lean/PrettyPrinter.lean Outdated Show resolved Hide resolved
@github-actions github-actions bot temporarily deployed to lean-lang.org/lean4/doc April 16, 2024 11:23 Inactive
leanprover-community-mathlib4-bot added a commit to leanprover-community/batteries that referenced this pull request Apr 16, 2024
leanprover-community-mathlib4-bot added a commit to leanprover-community/mathlib4 that referenced this pull request Apr 16, 2024
@kim-em kim-em added this pull request to the merge queue Apr 19, 2024
Merged via the queue into master with commit d1a42aa Apr 19, 2024
13 checks passed
@kim-em kim-em deleted the MessageData.ofConst branch April 19, 2024 04:06
kmill added a commit to kmill/lean4 that referenced this pull request Jul 5, 2024
In leanprover#3911, a refactor to share `MessageData` code between `ppConst` and the signature pretty printer unintentionally caused the signature pretty printer to use the `pp.tagAppFns` option. This causes, for example, `+` in `a + b` to independently have its own hover information.

This affects `#check` and was reported by Kevin Buzzard [on Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/degraded.20hover.20experience.20on.20.23check/near/449380674).
github-merge-queue bot pushed a commit that referenced this pull request Jul 5, 2024
In #3911, a refactor to share `MessageData` code between `ppConst` and
the signature pretty printer unintentionally caused the signature pretty
printer to use the `pp.tagAppFns` option. This causes, for example, `+`
in `a + b` to independently have its own hover information due to the
fact that `notation` app unexpanders use the head function's syntax as
the `ref` when constructing the notation syntax. This behavior of
`pp.tagAppFns` is intentional, and it is used by docgen, but it should
not be activated for signatures.

This affects `#check` and was reported by Kevin Buzzard [on
Zulip](https://leanprover.zulipchat.com/#narrow/stream/270676-lean4/topic/degraded.20hover.20experience.20on.20.23check/near/449380674).

This PR also makes sure the initial `ref` when applying app unexpanders
is `.missing`, rather than whatever random value might be present in the
`CoreM` context.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaks-mathlib This is not necessarily a blocker for merging: but there needs to be a plan 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