-
Notifications
You must be signed in to change notification settings - Fork 111
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: add lemma code-action #625
feat: add lemma code-action #625
Conversation
awaiting-review |
While I would prefer that |
Downstream projects don't have to use mathlib, you can declare the lemma command in one line given the implementation here. It could even be an option. But recommending to import mathlib is useful in case this was a typo - this is in fact the whole point of adding the lemma keyword to std, to improve the error message you get when using |
I view it as follows. |
When you say it's easy to turn on lemmas, you're only measuring the number of lines of code needed. But it is not discoverable and nothing else works that way. At least, the error message should say how to turn on lemmas. I still don't understand the utility of throwing a special error message if lemma is not implemented. The main effect I see are users asking "why is theorem better than lemma?" or "how do I turn on corollaries?" on Zulip. Why shouldn't Std just provide the lemma command and let users decide whether to use it or not? |
I think that the argument against I am happy to add a line to the error mentioning how to turn on lemmas. This is the most common use-case of the error, for me. Some mathematician who has no idea of what a programming language is, finds a tutorial with |
I think this sets a bad precedent, if we are not allowed to implement error messages for things that are not in the grammar but are known to be common misspellings. These are essentially independent design questions (or at least they should be independent):
I don't like the implication that an attempt to implement 2 requires us to implement 1 as well, or contrapositively, if we say "no" to 1 then we cannot do anything to improve 2. |
Honestly, I think it would be great if one could paste well formed Coq code using |
So lemma is a red herring... That makes sense! I couldn't understand the fixation on lemma. I can definitely get behind a solution that catches common misspellings and such, proposes alternatives and a code action to boot! I'm still perplexed by the implementation of the current (partial) solution. We actually are providing an implementation for lemma as a synonym for theorem, then we hide it behind a switch. That's unusual but worse is that keeping the switch off has no real benefit for users. |
The reason is because we know that, in the contexts where it works, In other words, good recovery from |
That makes sense. I still think just enabling lemma is a better experience than this for users but I understand that it shouldn't be part of the grammar. I'm happy to move on to more practical matters. Let's talk about that switch though. It's unique in design and not easily discoverable. How about making it an option that could be controlled locally using
I don't see disadvantages of using that mechanism. |
I just edited to use the option |
I personally do not have a preference between the initial version and the new I suspect that most people who will try to use |
Bummer. Looks like options that aren't declared in core (or plugins?) can't be set in the lakefile/command line. Using option won't work well for Mathlib unless that changes: set_option would have to be used in each file that uses lemma rather than being set globally in the lakefile. I think this is a bug with the option mechanism or a bug in my understanding how it should work. Reverting to the old switch is probably best for now but let's see what @joehendrix and @digama0 have to say. PS: It looks like putting this in a plugin could work but I'm not sure. |
I've been planning to write a fix for that for some time. Please report it as an issue so we can track it; I'm pretty sure that we can get consensus that this is undesirable, it's just a matter of designing a better solution. The reason this is the case is that lean needs to know the type of each option to typecheck the inputs for it, but maybe we can delay the typechecking for unknown options until after imports have been processed. |
Issue filed at lean4#3403. |
Lean core PR #4741 may finally enable this to work! |
889f27a
to
e7c8075
Compare
lean4#4741 has landed in v4.11.0-rc1. This PR is now ready for a final review and hopefully a quick merge! |
The 'lemma' command elaborates as 'theorem' but gives warning + codeaction to use 'theorem' instead.
I am picking up @jcommelin's PR #413, but I did not know how to modify the old one, so I am opening a new one.