-
Notifications
You must be signed in to change notification settings - Fork 662
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
Fix feedback levels #10612
Fix feedback levels #10612
Conversation
Should we we kill msg_info? What is the legitimate use case? (wondering about making its type different, to avoid confusion) |
I don't think so.
The one described in the API, that is information meant to be displayed in verbose mode (typically when running
The two functions are essentially doing the same thing, but correspond to different degrees of verbosity. I don't see what distinguishing them by typing would mean, TBH. |
b42e060
to
0d140c8
Compare
My impression is that one of the two is used rarely, so maybe a single function with a Anyway, the few examples of msg_notice you gave me (in a private chat) were variants of "foo is defined" that is not very useful, hence my questioning the entire thing. |
0d140c8
to
233f5a5
Compare
233f5a5
to
2d40331
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the debug output in auto
and even in ltac_profile
should be priority "debug", certainly at most Info
not Notice
.
You mean the output of As per documentation, "info" essentially means that it will be displayed by coqtop's verbose mode only. Why wouldn't IDEs want to show the result of the ltac profiler? |
Indeed I guess that having both notice and info is a bit confusing, OK then for notice in this case. Ultimately trace information seems specialized enough as to have it's own request. |
Most protocols tend to have a single information notification, however depending how you set the verbose level in the server more messages are generated then. This is normal as otherwise now all the info call are wasteful in the sense that they shouldn't run in the first place. |
I agree with that, but for now it seems a "notice" is the closest indeed.
I also agree. It is sometimes done this way today, but via ad-hoc guards to printer calls (testing if we are in verbose/debug mode, etc). It should certainly be consolidated, but it is out of scope for this modest PR. |
Any takers? @ejgallego maybe? |
@gares can you take this one? |
Ack-by: ejgallego Reviewed-by: gares
We make sure that
msg_notice
is used for command-like queries (Print
,Search
,...), as documented in the Feedback API.This is useful for IDEs, see e.g. coq-community/vscoq-legacy#35