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: 'Try this' support #130

Merged
merged 8 commits into from
May 14, 2023
Merged

feat: 'Try this' support #130

merged 8 commits into from
May 14, 2023

Conversation

digama0
Copy link
Member

@digama0 digama0 commented May 8, 2023

Adds proper widget / code action support for "try this" prompts in tactics. Still open: merging the widget into the message itself, depends on leanprover/lean4#2064 .

@digama0 digama0 mentioned this pull request May 8, 2023
1 task
@digama0 digama0 mentioned this pull request May 9, 2023
2 tasks
@fpvandoorn
Copy link
Member

fpvandoorn commented May 9, 2023

Two more things I noticed:

  • The tryThisAt in your original code snippet doesn't produce a blue squiggle under the tactic. Is it possible to add this (currently I also use a logInfoAt to produce the squiggle)
  • In the implementation of recommend using h I wrote, I put the logInfoAt and tryThisAt both on the word recommend. If I have my pointer at the end of the line, I still see the logInfoAt message that was created on the same line (without opening "All messages"). However, to see the tactic replacements, I need to have my pointer exactly on the word recommend (see screenshot below). Ideally it would show it whenever you are on the same line.
    image

@digama0
Copy link
Member Author

digama0 commented May 10, 2023

@fpvandoorn See whether the new version addresses your concerns. The widget hover (and code action) now happens as long as your cursor is over any of the lines containing the message, same as the messages display.

Copy link
Collaborator

@Vtec234 Vtec234 left a comment

Choose a reason for hiding this comment

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

Looks good, all functional issues with the PR that we identified are to be fixed in core rather than here.

Std/Tactic/TryThis.lean Outdated Show resolved Hide resolved
Std/Tactic/TryThis.lean Outdated Show resolved Hide resolved
Std/Tactic/TryThis.lean Outdated Show resolved Hide resolved
digama0 and others added 3 commits May 12, 2023 23:56
Co-authored-by: Wojciech Nawrocki <wjnawrocki+gh@protonmail.com>
Co-authored-by: Wojciech Nawrocki <wjnawrocki+gh@protonmail.com>
@digama0 digama0 merged commit c14108a into main May 14, 2023
@digama0 digama0 deleted the trythis branch May 14, 2023 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants