-
-
Notifications
You must be signed in to change notification settings - Fork 645
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
Timbre logs can show in the minibuffer #3587
Comments
Are you on Timbre as well? |
Yes. |
(Mostly a note to self) The cause might be that Clojure compilation errors and Timbre errors are both captured under a regex. So maybe stuff that was exclusively meant for Clojure compilation errors is also being triggered for Timbre. |
I can repro the minibuffer output issue, but not the overlay issue My attempt was the running following deftest: (ns timbre
(:require
[clojure.test :as t]
[taoensso.timbre :as log]))
(log/error 1)
(t/deftest foo
(t/is (zero? 1))) (NOTE: for best results, modify the buffer each time before running the tests) Could you please describe repro steps? |
One thing I've noticed at the nrepl-messages level is that replies with Timbre err output correspond to overly old requests:
Note how the request has id "22" but the error reply is for "15" - quite a few messages behind. It's as if the original We haven't changed stderr handling at nrepl level. We have #3206 though. I don't think this issue is releated to the overlay improvements since there's also the minifbuffer issue. I'll see what happens if I downgrade Timbre. |
I've created a simple project with the following test: (ns com.example.app-test
(:require
[clojure.test :refer [deftest is]]
[taoensso.timbre :as timbre]))
(deftest foo-test
(try
(throw (ex-info "Some error" {}))
(catch Exception e
(timbre/error e (ex-message e))))
(is (= {:one 1
:two 2
:three 3
:five 5
:six 6}
{:four 4}))) My CIDER settings: (setopt nrepl-hide-special-buffers nil
nrepl-use-ssh-fallback-for-remote-hosts t
cider-eldoc-display-context-dependent-info t
cider-font-lock-max-length 1000
cider-repl-display-help-banner nil
cider-repl-use-content-types t
cider-repl-wrap-history t
cider-repl-pop-to-buffer-on-connect 'display-only
cider-show-error-buffer 'except-in-repl
cider-auto-mode nil
cider-use-tooltips nil
cider-enrich-classpath t) |
nrepl messages
|
I tried that just now and couldn't repro it. Does it happen with a stock emacs and empty clojure project with no user profile? |
Thanks! Will check out again |
FYI I think it's this code Lines 915 to 920 in 21cdfeb
The surrounding lambda is what we call a It intends to operate over errors passed as nrepl response attributes. I've seen this work as intended for many months now - stderr works, and errors show as an overlay. If you wish to give it a shot, that would be most welcome as I cannot immediately reproduce the issues you're experiencing. |
vemv ***@***.***> writes:
FYI I think it's this code
https://github.com/clojure-emacs/cider/blob/21cdfeb62035154519cf9811b217487b6219915f/cider-eval.el#L915-L920
The surrounding lambda is what we call a `stderr-handler`. But it
certainly doesn't mean to show an overlay for every line of stderr.
It intends to operate over errors passes as nrepl response
attributes.
I've seen this work as intended for many months now - stderr works,
and errors show as an overlay.
If you wish to give it a shot, that would be most welcome as I
cannot immediately reproduce the issues you're experiencing.
Thanks. I'll try to dig into it when I have some time. From my current
understanding, it might be not CIDER's fault, but maybe cider-nrepl or
nrepl, I don't think this response should be sent in the first place.
From the nrepl-messages:
Request:
(-->
id "11"
op "load-file"
session "6c6c3a29-73ef-4786-b413-aa19af884873"
time-stamp "2024-01-17 09:41:53.574781000"
file "(ns com.example.app-test
(:require
[clojure.test :refer..."
file-name "app_test.clj"
file-path
"/Users/rrudakov/Personal/Projects/clj-bug/test/com/example/a..."
)
Response:
(<--
id "11"
session "6c6c3a29-73ef-4786-b413-aa19af884873"
time-stamp "2024-01-17 09:41:58.793757000"
err "This is an error
"
)
…--
Best regards, Roman
|
Thanks! Maybe it wasn't quite safe to assume that I'd wager that So, we could change the Just 1LOC before, there's a Which is to mean, thankfully the necessary data is already there, computed for us. Will see what I can do with that - I probably don't need more investigation atm. |
vemv ***@***.***> writes:
Will see what I can do with that - I probably don't need more
investigation atm.
Sounds good! :)
--
Best regards, Roman
|
https://clojurians.slack.com/archives/C099W16KZ/p1700123927174669
Partial workaround:
(setq inhibit-message t)
The text was updated successfully, but these errors were encountered: