Coq: run silently and explicitly Show when necessary - second attempt#762
Conversation
b7cb334 to
0758343
Compare
|
@Matafou , @erikmd , @cpitclaudel : With the option |
|
Hi @hendriktews! (I just quickly skimmed the PR code but) LGTM, thanks.
Agree |
|
Maybe I need to test how it interacts with debug mode. I don't have much time this week though. |
|
@hendriktews can you tell us what remains to be tested/adapted pls? I think this PR should not rot. |
|
I use this code on a daily basis, so it is working for me. Remaining things that I know:
|
I can test this.
I think this is a problem. I must admit I am always in 3 windows mode. |
So it is not a problem for you. It is only a problem when there is no window showing the response buffer when you issue the command. Note that any nonempty response from the search command is correctly shown. Meanwhile I discovered another problem:
|
No it is not a problem for me, but the lack of user feedback in this case is a (small) problem.
Indeed. It also happens after other messages like "This subproof is complete, ...". The reason is that the search command is now followed by a I tested and adding "No more goals." to |
942720d to
837fae8
Compare
5bdedad to
cf3398a
Compare
|
I support merging this now. |
|
OK, let's wait for another few days, in case somebody else wants to try it. |
|
Please excuse the delay, I thought it would be good to first add tests for the searches that get broken by this PR. Hopefully they are now ready, see #827 |
This is a step towards fixing ProofGeneral#568. It fixes the cases after Proof, comments, auto, errors, Search and Check, leaving now 2 instead of 8 failing tests in ci/simple-tests/test-goals-present.el. Admitted is not handled correctly any more, which is a regression. Using proof-shell-handle-delayed-output-hook and proof-shell-handle-error-or-interrupt-hook we issue a Show command as a priority action item when the last (normal) action item has been processed. The new action item flag 'keep-response tells the generic machinery to not clear the response buffer and to keep it present in two-pane mode in case an error was detected or the last command was a Search or Check that produced a response. The new action item flag 'dont-show-when-silent is used to distinguish the additional Show commands and to avoid an endless loop of Show commands. Set proof-shell-last-output-kind now in proof-shell-handle-delayed-output such that it correctly reflects the cases of goals and response (which has not been the case since commit 037dc9b from 2009. This commit breaks coq-show-proof-stepwise to some extend. Expect 080_coq-test-regression-show-proof-stepwise to fail. Additionally: - update manuals - expect errors in tests 020_coq-test-definition, 090_coq-test-regression-Fail and 091_coq-test-regression-Fail because messages are not printed in silent mode
This solves the remaining known problems from ProofGeneral#568. Fixes ProofGeneral#568.
Add user option coq-run-completely-silent, which, when disabled, switches Coq to old behavior where Coq is dynamically switched to silent on longer action item lists.
- Inside the urgent prooftree action, call now the proof assistant specific function `proof-tree-assistant-specific-urgent-action'. For Coq/Rocq this adds a show command which calls `proof-tree-display-goal-callback' in its callback, which processes the output and sends appropriate commands to prooftree. - Move the call of the urgent prooftree action before calling adding `proof-shell-empty-action-list-command', such that these last actions happen after the show command. - In `proof-shell-filter-manage-output', save the end of the previous command in new variable `proof-shell-old-proof-marker-position' such that callbacks running after the next command has been sent to Coq can access it. This is used inside `proof-tree-display-goal-callback'. - proof-tree display is now only supported when Coq runs completely silent. Supporting it in the other mode should not be too difficult, but I don't think it is worth the effort.
|
The last update rebases this PR onto #827 and marks some of these new tests as expected to fail. If nobody objects, I'll merge this Friday. |
| ;; (coq-should-buffer-regexp (regexp-quote "The command has indeed failed with message: Tactic failure: Cannot solve this goal.") "*response*") | ||
|
|
||
| (ert-deftest 091_coq-test-regression-Fail() | ||
| ;; XXX What is the difference between this test and |
There was a problem hiding this comment.
The point in coq file is not the same: "(FailNoTrace)" vs "(FailTrace)". Concretelty the difference is the command Local Unset Ltac Backtrace..
There was a problem hiding this comment.
OK, it would be nice if the documentation in that file would describe what is tested there.
|
|
||
| (ert-deftest 020_coq-test-definition () | ||
| ;; There are no infomsgr when running silent. | ||
| :expected-result :failed |
There was a problem hiding this comment.
We should probably test this when only one command is sent at a time.
There was a problem hiding this comment.
AFAICT there is only one command processed in this test. Even if you process one definition alone, there is no infomsg when running silent.
There was a problem hiding this comment.
You are right. this behaviour is ok for me. I wonder if someone really care about this fonfirmation messages outside pure textual coqtop use.
|
This works like a charm. Thanks a lot @hendriktews for this work. |
|
After a few days of work the only annoying thing for me is the "Set some option; do the command; unset the option" triggered by the C-u prefix. Like |
|
Do you mean |
|
Right! I forgot to use the fix. |
|
FWIW, I found the suppression of infomsg really bad. As a user I'm expecting them during interactive sessions. |
|
Please open an issue that describes under which situations you miss a message. Then we can add a test and maybe fix it. |
|
Here it is: #842 but it's basically all over the place, not a single instance. Virtually any command can produce a worthwile infomsg. |
This is another attempt towards fixing #568.