zed: Hide log file actions when logging to stdout - #57114
Conversation
f5b6932 to
cb23ee4
Compare
dinocosta
left a comment
There was a problem hiding this comment.
Hey @tredondo ! 👋
I've chatted about this with the team and, although this seems like a valid UX concern, as we're displaying actions in the command palette which will lead to an error when run, we believe we don't want to always be creating this file.
As a suggestion, we can probably update zed::init such that the action handlers for both OpenLog and RevealLogInFileManager are only registered if crate::stdout_is_a_pty is false, that way those two actions will not show up in the command palette.
Feel free to update your changes 🙂
cb23ee4 to
6fd4b66
Compare
`zed: open log` and `zed: reveal log in file manager` operate on `Zed.log`, which is only written when Zed logs to a file. When stdout is a PTY (e.g. running the `zed-editor` binary or `cargo run` directly from a terminal), logs go to stdout and the file is never created, so both actions failed with "Unable to access/open log file...: No such file or directory". Only register the two action handlers when stdout isn't a PTY, so they no longer appear in the command palette instead of showing up and erroring when invoked. Release Notes: - Fixed `zed: open log` and `zed: reveal log in file manager` erroring when Zed was launched directly from a terminal; they are now hidden in that case, since logs go to stdout rather than a file.
6fd4b66 to
f0b5a48
Compare
|
@dinocosta Made the change, sorry it took so long. |
|
@dinocosta: happy to help! Any chance to take a look at #57376? It's also small (a dozen lines). |
|
@dinocosta I'm having a second though about the final version of this PR: it worsens observability and makes forensic investigations impossible after the terminal window was closed. I was trying to figure out what clobbered the mtime of hundreds of files in my repo, and Zed's log would've been key in solving that mystery. However, because I'm running the dev build, all logging when to the terminal and not to any file. In the intervening days, the terminal was closed and logs were lost. Could the team revisit this?
My first fix prevented those errors, because the log file was always created. |
I'll see if I can have someone more familiar with our Markdown-rendering take a look at it. I can see it's already been labelled appropriately, so someone should eventually take a look 🙂
Believe we'll want to keep it as is, as Zed does not currently take the release channel into consideration for the log file. This would mean that the logs from a development build would clutter the log file for the stable release. Creating the log file is also not enough, as we'd need to change how Zed decides whether to output to the terminal or the log file to now output to both when running a development build. Having said that, there's still a couple of ways you could persist those logs:
Hope that helps! |
When running `zed` [directly from a foreground terminal](zed-industries#51351 (comment)) (stable `zed-editor`, or zed dev builds via `cargo run` as in zed-industries#51351), Zed only enables the stdout log sink and never creates `Zed.log` on disk. This causes `zed: open log` and `zed: reveal log in file manager` to always failed with `Unable to access/open log file ...: Failed to read file... No such file or directory (os error 2)`. Per maintainer feedback, rather than always creating the log file, this change only registers the `OpenLog` and `RevealLogInFileManager` action handlers when stdout isn't a PTY. In the PTY case the two actions no longer appear in the command palette at all, so they can't be invoked only to error. Repro (before this change): 1. `rm -rf ~/.local/share/zed/logs/` 2. `~/.local/zed.app/libexec/zed-editor` (the GUI binary directly, bypassing the CLI wrapper which detaches and sets `ZED_FORCE_CLI_MODE`) 3. In Zed, run `zed: open log` → error toast Release Notes: - Fixed `zed: open log` and `zed: reveal log in file manager` appearing and erroring when Zed was launched directly from a terminal. These actions are now hidden in that scenario, since logs go to stdout rather than Zed's log file. --------- Co-authored-by: dino <dinojoaocosta@gmail.com>
When running `zed` [directly from a foreground terminal](zed-industries#51351 (comment)) (stable `zed-editor`, or zed dev builds via `cargo run` as in zed-industries#51351), Zed only enables the stdout log sink and never creates `Zed.log` on disk. This causes `zed: open log` and `zed: reveal log in file manager` to always failed with `Unable to access/open log file ...: Failed to read file... No such file or directory (os error 2)`. Per maintainer feedback, rather than always creating the log file, this change only registers the `OpenLog` and `RevealLogInFileManager` action handlers when stdout isn't a PTY. In the PTY case the two actions no longer appear in the command palette at all, so they can't be invoked only to error. Repro (before this change): 1. `rm -rf ~/.local/share/zed/logs/` 2. `~/.local/zed.app/libexec/zed-editor` (the GUI binary directly, bypassing the CLI wrapper which detaches and sets `ZED_FORCE_CLI_MODE`) 3. In Zed, run `zed: open log` → error toast Release Notes: - Fixed `zed: open log` and `zed: reveal log in file manager` appearing and erroring when Zed was launched directly from a terminal. These actions are now hidden in that scenario, since logs go to stdout rather than Zed's log file. --------- Co-authored-by: dino <dinojoaocosta@gmail.com>
When running
zeddirectly from a foreground terminal (stablezed-editor, or zed dev builds viacargo runas in #51351), Zed only enables the stdout log sink and never createsZed.logon disk. This causeszed: open logandzed: reveal log in file managerto always failed withUnable to access/open log file ...: Failed to read file... No such file or directory (os error 2).Per maintainer feedback, rather than always creating the log file, this change only registers the
OpenLogandRevealLogInFileManageraction handlers when stdout isn't a PTY. In the PTY case the two actions no longer appear in the command palette at all, so they can't be invoked only to error.Repro (before this change):
rm -rf ~/.local/share/zed/logs/~/.local/zed.app/libexec/zed-editor(the GUI binary directly, bypassing the CLI wrapper which detaches and setsZED_FORCE_CLI_MODE)zed: open log→ error toastRelease Notes:
zed: open logandzed: reveal log in file managerappearing and erroring when Zed was launched directly from a terminal. These actions are now hidden in that scenario, since logs go to stdout rather than Zed's log file.