Skip to content

[Merged by Bors] - fix: support more parsers in #help#12287

Closed
digama0 wants to merge 2 commits intomasterfrom
get_head_tk
Closed

[Merged by Bors] - fix: support more parsers in #help#12287
digama0 wants to merge 2 commits intomasterfrom
get_head_tk

Conversation

@digama0
Copy link
Copy Markdown
Member

@digama0 digama0 commented Apr 20, 2024

As reported on Zulip, the #help command is out of date with respect to the parsers that can be obscuring the search for the "first token", used to power the search-by-token-prefix form of the command #help tactic "measur". The list below has been tested against everything in lean, and the only things that fail to find a first token now are those that actually don't have a token like the ident parser.


Open in Gitpod

@grunweg grunweg added the awaiting-author A reviewer has asked the author a question or requested changes. label May 24, 2024
@grunweg
Copy link
Copy Markdown
Contributor

grunweg commented May 24, 2024

AIUI, this is blocked on the author fixing the build failures - let me label it as such.

\n\
\nTry this:\
\nset_option trace.Meta.synthInstance true\
\n#synth SlimCheck.Testable ({tgt'})"
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The reason this is here is because fixing #help caused the set of hash commands used by lint_hash_commands to include #synth, which now causes a false positive on this line. I fixed it by indenting the string literal so it doesn't look like an actual #synth call.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think #13142 should fix this: the new Lean version of this linter ought to see this is a string literal.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I just ran an experiment: #13230 shows that building mathlib succeeds (in particular, the syntax linter does not flag #synth), but the text-based one works.

I think that the #-command CI checks should be removed.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't understand what you are trying to say. Prior to this commit, #help would not flag #synth as a hash command, which causes the text-based CI to produce false negatives for all #synth commands. After it, #synth is treated as a hash command, producing a false positive right here in a string literal. I think the string literal is better anyway with proper indentation. The new syntax linter does not use #help to begin with and just fires on all atoms starting with # (which has its own set of false negatives since it doesn't recurse into subexpressions correctly, basically reproducing the error fixed in this PR in a new implementation, in addition to the FN because it doesn't consider nested commands like set_option ... in #synth).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What I was trying to say is that it is only the string-based, #help-dependent linter that flags this, not the more recent syntax linter. Hence, removing the older string-based linter would make this change on Slimcheck not needed. Of course, the change could still be desirable!

@digama0 digama0 added awaiting-review and removed awaiting-author A reviewer has asked the author a question or requested changes. labels May 25, 2024
@urkud urkud added the t-meta Tactics, attributes or user commands label Jun 7, 2024
@robertylewis
Copy link
Copy Markdown
Member

bors merge

@github-actions github-actions bot added ready-to-merge This PR has been sent to bors. and removed awaiting-review labels Jun 19, 2024
mathlib-bors bot pushed a commit that referenced this pull request Jun 19, 2024
As [reported on Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/missing.20doc.20of.20.60.23check_failure.60/near/434466058), the `#help` command is out of date with respect to the parsers that can be obscuring the search for the "first token", used to power the search-by-token-prefix form of the command `#help tactic "measur"`. The list below has been tested against everything in lean, and the only things that fail to find a first token now are those that actually don't have a token like the ident parser.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
@mathlib-bors
Copy link
Copy Markdown
Contributor

mathlib-bors bot commented Jun 19, 2024

Pull request successfully merged into master.

Build succeeded:

@mathlib-bors mathlib-bors bot changed the title fix: support more parsers in #help [Merged by Bors] - fix: support more parsers in #help Jun 19, 2024
@mathlib-bors mathlib-bors bot closed this Jun 19, 2024
@mathlib-bors mathlib-bors bot deleted the get_head_tk branch June 19, 2024 11:32
AntoineChambert-Loir pushed a commit that referenced this pull request Jun 20, 2024
As [reported on Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/missing.20doc.20of.20.60.23check_failure.60/near/434466058), the `#help` command is out of date with respect to the parsers that can be obscuring the search for the "first token", used to power the search-by-token-prefix form of the command `#help tactic "measur"`. The list below has been tested against everything in lean, and the only things that fail to find a first token now are those that actually don't have a token like the ident parser.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
grunweg pushed a commit that referenced this pull request Jun 20, 2024
As [reported on Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/missing.20doc.20of.20.60.23check_failure.60/near/434466058), the `#help` command is out of date with respect to the parsers that can be obscuring the search for the "first token", used to power the search-by-token-prefix form of the command `#help tactic "measur"`. The list below has been tested against everything in lean, and the only things that fail to find a first token now are those that actually don't have a token like the ident parser.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
kbuzzard pushed a commit that referenced this pull request Jun 26, 2024
As [reported on Zulip](https://leanprover.zulipchat.com/#narrow/stream/113488-general/topic/missing.20doc.20of.20.60.23check_failure.60/near/434466058), the `#help` command is out of date with respect to the parsers that can be obscuring the search for the "first token", used to power the search-by-token-prefix form of the command `#help tactic "measur"`. The list below has been tested against everything in lean, and the only things that fail to find a first token now are those that actually don't have a token like the ident parser.



Co-authored-by: Mario Carneiro <di.gama@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-merge This PR has been sent to bors. t-meta Tactics, attributes or user commands

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants