-
Notifications
You must be signed in to change notification settings - Fork 90
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
Compile before require does not ignore comments #352
Labels
Comments
Thanks for reporting. |
@hendriktews can you have a look at it pls?
Yes, I try.
H.
|
Vincent,
thanks for reporting and recording this. The problem is that I
extract the module names with a simple regular expression from
the command string. I just wonder if we could run coqdep on the
original require command string and let coqdep struggle with
nested comments and strings inside them (instead of correctly
parsing them in elisp). I need some more time to think about
this.
Surprisingly, things work as expected when comments are in Japanese;
This is because the default regular expression for module names
does not match Japanese characters.
Hendrik
|
hendriktews
added a commit
to hendriktews/PG
that referenced
this issue
Nov 1, 2020
- no clones any more, existing jobs are directly linked - the whole require command is processed by coqdep to determine the required files, this fixes ProofGeneral#352 - the require commands are a separate kind of jobs, because they do not need to get compiled - queue items are only stored in require jobs and file jobs can not have a queue dependency, this simplifies the logic
hendriktews
added a commit
to hendriktews/PG
that referenced
this issue
Nov 7, 2020
- no clones any more, existing jobs are directly linked - the whole require command is processed by coqdep to determine the required files, this fixes ProofGeneral#352 - the require commands are a separate kind of jobs, because they do not need to get compiled - queue items are only stored in require jobs and file jobs can not have a queue dependency, this simplifies the logic
I just ran into this as well -- it took me some time to realize why PG is failing on this line:
|
hendriktews
added a commit
to hendriktews/PG
that referenced
this issue
Dec 6, 2020
- no clones any more, existing jobs are directly linked - the whole require command is processed by coqdep to determine the required files, this fixes ProofGeneral#352 - the require commands are a separate kind of jobs, because they do not need to get compiled - queue items are only stored in require jobs and file jobs can not have a queue dependency, this simplifies the logic
hendriktews
added a commit
to hendriktews/PG
that referenced
this issue
Dec 7, 2020
- no clones any more, existing jobs are directly linked - the whole require command is processed by coqdep to determine the required files, this fixes ProofGeneral#352 - the require commands are a separate kind of jobs, because they do not need to get compiled - queue items are only stored in require jobs and file jobs can not have a queue dependency, this simplifies the logic
hendriktews
added a commit
to hendriktews/PG
that referenced
this issue
Dec 18, 2020
- no clones any more, existing jobs are directly linked - the whole require command is processed by coqdep to determine the required files, this fixes ProofGeneral#352 - the require commands are a separate kind of jobs, because they do not need to get compiled - queue items are only stored in require jobs and file jobs can not have a queue dependency, this simplifies the logic
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When there are comments in a
Require
orRequire Import
sentence, and when “Compile Before Require” is enabled, Proof-General tries to compile the modules whose names are in comment. This behavior is unexpected and may fail (maybe because the comment is an actual comment, not a list of module names).Test case:
Surprisingly, things work as expected when comments are in Japanese; e.g., the following does not fail.
Tested with current master (a789470) and Coq 8.7.2.
The text was updated successfully, but these errors were encountered: