linux: Fix file not opening from file explorer #21137
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
maxdeviant
changed the title
maxdeviant
changed the title
SomeoneToIgnore
added
the
cla-signed
SomeoneToIgnore
approved these changes
Nov 29, 2024
JaLnYn
pushed a commit
to JaLnYn/zed
that referenced
this pull request
Nov 29, 2024
Closes zed-industries#20070 Release Notes: - Fixed issue where files wouldn't open from the file explorer. - Fixed "Open a new workspace" option on the desktop entry right-click menu. Context: Zed consists of two binaries: - `zed` (CLI component, located at `crates/cli/main.rs`) - `zed-editor` (GUI component, located at `crates/zed/main.rs`) When `zed` is used in the terminal, it checks if an existing instance is running. If one is found, it sends data via a socket to open the specified file. Otherwise, it launches a new instance of `zed-editor`. For more details, see the `detect` and `boot_background` functions in `crates/cli/main.rs`. Root Cause: Install process creates directories like `.local/zed.app` and `.local/zed-preview.app`, which contain desktop entries for the corresponding release. For example, `.local/zed.app/share/applications` contains `zed.desktop`. This desktop entry includes a generic `Exec` field, which is correct by default: ```sh Comment=A high-performance, multiplayer code editor. TryExec=zed StartupNotify=true ``` The issue is in the `install.sh` script. This script copies the above desktop file to the common directory for desktop entries (.local/share/applications). During this process, it replaces the `TryExec` value from `zed` with the exact binary path to avoid relying on the shell's PATH resolution and to make it explicit. However, replacement incorrectly uses the path for `zed-editor` instead of the `zed` CLI binary. This results in not opening a file as if you use `zed-editor` directly to do this it will throw `zed is already running` error on production and open new instance on dev. Note: This PR solves it for new users. For existing users, they will either have to update `.desktop` file manually, or use `install.sh` script again. I'm not aware of zed auto-update method, if it runs `install.sh` under the hood.
1 task
github-merge-queue bot
pushed a commit
that referenced
this pull request
Dec 22, 2024
… from .desktop file (#22335) Closes #21406 Context: A few weeks ago on Linux, we resolved an [issue](#20070) where users could not open more than one file from the file explorer. This was fixed by replacing `zed-editor` (zed binary in the code) with `zed` (cli binary in the code) in the `.desktop` file. The reason for this change was that using the cli to open files is more convenient - it determines weather to spawn a new Zed instance or use an existing one, if we use main binary instead it would throw error `Zed is already running`. You can read the complete PR here: [linux: Fix file not opening from file explorer](#21137). While this fix resolved the original issue, it introduced a new one. Problem: When the cli binary is used, it assumes it is always being invoked from a terminal and relies on `std::env::vars()` to retrieve the environment variables needed to spawn Zed. These env vars are then passed to the worktree, and eventually, languages use the `PATH` from this env to find binaries. This leads to the "Failed to start language server" error when the `.desktop` entry is used on Linux. Solution: When the `zed-editor` binary is used, it uses some clever Unix-specific logic to retrieve the default shell (`load_shell_from_passwd`) and then fetch the env vars from that shell (`load_login_shell_environment`). This same logic should be used in the cli binary when it is invoked via a `.desktop` entry rather than from a terminal. Approach: I moved these two functions mentioned above to a utils file and reused them in cli binary to fetch env vars only on Linux when it is not run from a terminal. This provides missing paths, and fix the issue. It is also possible to handle this in the `zed-editor` binary by modifying the logic in `handle_cli_connection`, where `CliRequest::Open` is processed. There we can discard incoming env, and use our logic. But discarding incoming envs felt weird, and I thought it's better to handle this at source. Release Notes: - Fixed `Failed to start language server` errors when starting from dekstop entry on Linux
CharlesChen0823
pushed a commit
to CharlesChen0823/zed
that referenced
this pull request
Dec 24, 2024
… from .desktop file (zed-industries#22335) Closes zed-industries#21406 Context: A few weeks ago on Linux, we resolved an [issue](zed-industries#20070) where users could not open more than one file from the file explorer. This was fixed by replacing `zed-editor` (zed binary in the code) with `zed` (cli binary in the code) in the `.desktop` file. The reason for this change was that using the cli to open files is more convenient - it determines weather to spawn a new Zed instance or use an existing one, if we use main binary instead it would throw error `Zed is already running`. You can read the complete PR here: [linux: Fix file not opening from file explorer](zed-industries#21137). While this fix resolved the original issue, it introduced a new one. Problem: When the cli binary is used, it assumes it is always being invoked from a terminal and relies on `std::env::vars()` to retrieve the environment variables needed to spawn Zed. These env vars are then passed to the worktree, and eventually, languages use the `PATH` from this env to find binaries. This leads to the "Failed to start language server" error when the `.desktop` entry is used on Linux. Solution: When the `zed-editor` binary is used, it uses some clever Unix-specific logic to retrieve the default shell (`load_shell_from_passwd`) and then fetch the env vars from that shell (`load_login_shell_environment`). This same logic should be used in the cli binary when it is invoked via a `.desktop` entry rather than from a terminal. Approach: I moved these two functions mentioned above to a utils file and reused them in cli binary to fetch env vars only on Linux when it is not run from a terminal. This provides missing paths, and fix the issue. It is also possible to handle this in the `zed-editor` binary by modifying the logic in `handle_cli_connection`, where `CliRequest::Open` is processed. There we can discard incoming env, and use our logic. But discarding incoming envs felt weird, and I thought it's better to handle this at source. Release Notes: - Fixed `Failed to start language server` errors when starting from dekstop entry on Linux
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #20070
Release Notes:
Context:
Zed consists of two binaries:
zed(CLI component, located atcrates/cli/main.rs)zed-editor(GUI component, located atcrates/zed/main.rs)When
zedis used in the terminal, it checks if an existing instance is running. If one is found, it sends data via a socket to open the specified file. Otherwise, it launches a new instance ofzed-editor. For more details, see thedetectandboot_backgroundfunctions incrates/cli/main.rs.Root Cause:
Install process creates directories like
.local/zed.appand.local/zed-preview.app, which contain desktop entries for the corresponding release. For example,.local/zed.app/share/applicationscontainszed.desktop.This desktop entry includes a generic
Execfield, which is correct by default:The issue is in the
install.shscript. This script copies the above desktop file to the common directory for desktop entries (.local/share/applications). During this process, it replaces theTryExecvalue fromzedwith the exact binary path to avoid relying on the shell's PATH resolution and to make it explicit.However, replacement incorrectly uses the path for
zed-editorinstead of thezedCLI binary. This results in not opening a file as if you usezed-editordirectly to do this it will throwzed is already runningerror on production and open new instance on dev.Note: This PR solves it for new users. For existing users, they will either have to update
.desktopfile manually, or useinstall.shscript again. I'm not aware of zed auto-update method, if it runsinstall.shunder the hood.