### Summary
This PR fixes three small bugs that could cause a multi-workspace's
reported location to be out of sync with its actual location, which
causes following to break in multiplayer collaboration sessions.
The first bug was that each child workspace's title bar within a
multi-workspace had its own `cx.observe_window_activation` subscription
that would set the associated child workspace's project location. This
caused problems because it ran for all child workspaces instead of just
the active workspace within the multi-workspace. The fix was moving
`cx.observe_window_activation` to the `cx.observe_new` call in the
`call` crate that tracks newly created windows/multi-workspaces.
The second bug was caused by an incorrect `if` statement in a
`MultiWorkspace` subscription that would return early if the window was
active and the event emitted by `MultiWorkspace` wasn't
`ActiveWorkspaceChanged`. This caused issues when the window was
inactive because it would incorrectly set the active call's location to
the wrong project. The fix was making the early return happen if the
window wasn't active.
The third bug was that every child `Workspace` also observed window
activation and called `update_active_view_for_followers`. This allowed
hidden workspaces to report their active view instead of only the active
workspace.
### Testing
I added a property test for bugs one/two and another prop test for bug 3
Release Notes:
- collab: Fix out of sync following bugs
- **Revert "livekit: Preserve tokens on channel rejoin (#59388)"**
- **Revert "call: Log LiveKit connection info refresh outcomes in retry
loop (#59205)"**
- **Revert "audio: Fix phantom presence in channels (#59195)"**
- **proto: Reserve RejoinRoomResponse field 4 after revert**
We've observed a spike of phantom collaborator issues after this fixes.
This sucks and I'm quite unhappy about it.
# Objective
- Describe the objective or issue this PR addresses.
- If you're fixing a specific issue, use "Fixes #X" for each issue as
[described in the GitHub
docs](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
## Solution
- Describe the solution used to achieve the objective above.
## Testing
- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
## Self-Review Checklist:
- [ ] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [ ] Tests cover the new/changed behavior
- [ ] Performance impact has been considered and is acceptable
## Showcase
> This section is optional. If this PR does not include a visual change
or does not add a new user-facing feature, you can delete this section.
- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
- A before/after comparison is very useful for changes to existing
features!
While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:
<details>
<summary>Click to view showcase</summary>
My super cool demos here
</details>
---
Release Notes:
- N/A
---------
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- **collab: Don't revoke LiveKit token when rejoining the same channel**
- **call: Bound LiveKit reconnect attempts and decouple from collab
rejoin**
# Objective
- Describe the objective or issue this PR addresses.
- If you're fixing a specific issue, use "Fixes #X" for each issue as
[described in the GitHub
docs](https://docs.github.com/en/issues/tracking-your-work-with-issues/using-issues/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword).
## Solution
- Describe the solution used to achieve the objective above.
## Testing
- Did you test these changes? If so, how?
- Are there any parts that need more testing?
- How can other people (reviewers) test your changes? Is there anything
specific they need to know?
- If relevant, what platforms did you test these changes on, and are
there any important ones you can't test?
## Self-Review Checklist:
- [ ] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [ ] Tests cover the new/changed behavior
- [ ] Performance impact has been considered and is acceptable
## Showcase
> This section is optional. If this PR does not include a visual change
or does not add a new user-facing feature, you can delete this section.
- Help others understand the result of this PR by showcasing your
awesome work!
- If this PR includes a visual change, consider adding a screenshot,
GIF, or video
- A before/after comparison is very useful for changes to existing
features!
While a showcase should aim to be brief and digestible, you can use a
toggleable section to save space on longer showcases:
<details>
<summary>Click to view showcase</summary>
My super cool demos here
</details>
---
Release Notes:
- N/A or Added/Fixed/Improved ...
Follow-up to #59195. While verifying that fix against prod we hit a log
that couldn't distinguish "retrying with a fresh token that is still
revoked" from "silently retrying with the same dead token because the
server returned no connection info" (e.g. a collab deploy predating
#59195). Make the retry loop self-diagnosing:
- Warn when `RejoinRoomResponse` carries no `live_kit_connection_info`,
since the loop then reuses the existing token.
- On refresh, log whether the token actually changed.
Release Notes:
- N/A
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
# Objective
You know that feeling? You join the channel, crack a banger joke to
break the ice - yet, nobody laughs. Worse yet, you're swarmed with Slack
messages like "Piotr, we can't hear you". After a while you realize that
maybe it's for the best as the joke was not as good as you've initially
thought, but still: it's possible to get into a state where other
participants see you as muted. You don't hear anybody. Bummer.
That's what happened when we were hoping to pair with @dinocosta today.
I could not hear him. However, he had some logs to share which indicated
that while collab thought he's in the room with me, live-kit was
rejecting his token as it was revoked.
What clanker helped us figure out is that there's a race-y code path
where if you disconnect from a given room abruptly, collab can put you
in a limbo.
<details><summary>Clanker's idea of what's going on</summary>
<p>
Found the whole story. Here's what those logs mean and where the bug
surface is.
What's happening
The error at room.rs:1809 is the detach_and_log_err on the task spawned
by spawn_room_connection (crates/call/src/call_impl/room.rs:1754-1810).
So this is the initial livekit::Room::connect call failing — the LiveKit
SDK internally retried 3 times (that's the retrying... (1/3) lines), got
401 invalid token: revoked every time, gave up, and the error propagated
out of the ? at line 1762 and was logged-and-dropped.
The "revoked" comes from the collab server: leave_room_for_session
(crates/collab/src/rpc.rs:4091-4095) calls
live_kit.remove_participant(livekit_room,
session.user_id().to_string()), and LiveKit Cloud treats a removed
participant's token as revoked. Crucially, the LiveKit identity is just
the user id, not the connection id, while the DB-side
leave_room(connection_id) is per-connection. leave_room_for_session
fires from three places:
1. Explicit leave (rpc.rs:1666)
2. connection_lost, after RECONNECT_TIMEOUT expires (rpc.rs:1342)
3. Stale-connection cleanup when re-joining a channel (rpc.rs:3401)
So there's a race: an old connection's delayed cleanup (path 2) can fire
after the same user has rejoined the room on a new connection and been
issued a fresh token — and remove_participant(room, user_id)
kicks/revokes the user's current LiveKit participant, because the
identity is shared across connections. Quick restart of Zed, two
instances, or a reconnect that straddles the server's RECONNECT_TIMEOUT
all set this up.
Client-side consequences (this connects to your first question)
When that initial connect fails:
- this.live_kit stays None and this.diagnostics is never created — lines
1780-1791 only run on success. So in this failure mode CallDiagnostics
doesn't exist at all; the "stats never overridden" path I described
earlier isn't even reached.
- There is no retry and no token refresh. The failure is
detach_and_log_err'd and that's the end of it. The user stays in the
room at the collab level (status is still Online, they appear in the
channel), but with no audio and no way to recover short of leaving and
rejoining.
- Even a collab-level reconnect doesn't help: RejoinRoomResponse
(crates/proto/proto/call.proto:58) carries no live_kit_connection_info,
and rejoin() never calls spawn_room_connection — the LiveKit connection
is established exactly once, in Room::new, with the token from the
original join response.
So the user-visible symptom is: you're "in" the call, everyone sees you,
but you can't hear or be heard, and there's no error surfaced in the UI
— just this log line.
If you want to fix it, the two angles are server-side (make
leave_room_for_session skip the LiveKit removal when the user still has
a live connection in that room, or scope LiveKit identities
per-connection) and client-side (treat a failed spawn_room_connection as
fatal-but-recoverable: surface it, and have rejoin request fresh
connection info instead of silently carrying on). Happy to dig into
either if you want.
</p>
</details>
So tl;dr: our token was swapped out underneath us. One way to "fix" it
was to just go into another channel, but that was a bummer.
We have a good and reliable repro for it though: `kill -9 $ZED_PID`
followed by an attempt to rejoin the same channel within 30s would
consistently put us in that state.
Put another way: you panic and thus do not send a clean "leave room"
message to LK. if you rejoin the channel within 30s (after restarting),
LK will invalidate the very token you're attempting to use (as it cleans
up old tokens).
It may also happen without crashing, but that's the most reliable way to
repro the issue. Long story short your LK token gets tainted and you
can't share the audio anyhow.
## Solution
The fix is both client and server-side.
On client's side, we now retry the reconnection with a back-off and we
grab a fresh token off of reconnect attempts.
On server's side, we share the current token on room reconnect attempts.
We've also tweaked the call diagnostics to not use `unwrap_or_default`
so much: it was hard for us to tell that we're in a totally bogus state
and we've only reached that conclusion based on source code analysis. We
now show some generic "ok you're in a borked state pls report a bug"
message instead.
## Testing
We added tests that present the flacky scenario. We've also tried to
setup an infra to test the new behaviour, but sadly, local LK instance
seems to revoke tokens more leniently than the prod..
OTOH, we are quite confident that once new collab is deployed, we'll be
able to mince the new tokens and life is going to be good. Even when
running against the current prod instance we could see that our code
changed the behaviour for the better, as now we'll actually try to use a
new token if one is provided by collab. For that to happen we need to
redeploy though.
## Self-Review Checklist:
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content adheres to Zed's UI standards
([UX/UI](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
and
[icon](https://github.com/zed-industries/zed/blob/main/crates/icons/README.md)
guidelines)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Release Notes:
- Fixed a race condition that caused collab users to not receive/send
any audio to their peers.
---------
Co-authored-by: dino <dinojoaocosta@gmail.com>
Co-authored-by: Dino <dino@zed.dev>
Co-authored-by: zed-zippy[bot] <234243425+zed-zippy[bot]@users.noreply.github.com>
The join sounds get annoying in large meetings, let's not play it
anymore when the meeting get's really big. The guest joined sound is
plays regardless of group size so participants get a heads-up when
someone external joins.
Self-Review Checklist:
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Closes #ISSUE
Release Notes:
- Improved join sound no longer plays in large meetings
Auto watch's lifespan should be tied to that of the call and it should
not be assumed the user wants to have this on indefinitely (until app
restart), as it's more of a niche feature. This PR disables it when the
user leaves a call.
Self-Review Checklist:
- [X] I've reviewed my own diff for quality, security, and reliability
- [X] Unsafe blocks (if any) have justifying comments
- [X] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [X] Tests cover the new/changed behavior
- [X] Performance impact has been considered and is acceptable
Release Notes:
- N/A
This PR renames the `UserId` type in the `client` crate to
`LegacyUserId`.
The `id` field on the `User` has also been renamed to `legacy_id`.
This is strictly a rename, no change in behavior.
Release Notes:
- N/A
This PR fixes a few bugs, updates some UI, and improves testing of auto
watch. It'll likely be easier to review commit by commit:
- Swapped the Copy Channel Link and Auto Watch buttons so Auto Watch
appears in a better position. The UI is still not great, but I think
this tweak will improve it until someone on design can help.
Before:
<img width="324" height="61"
alt="589131021-c967dfe1-9026-4a1d-a399-b735303f2de0"
src="https://github.com/user-attachments/assets/7cd414cd-5a13-4e16-ab6e-5de6d2cd64ed"
/>
After:
<img width="373" height="77"
alt="589131282-607e15a5-e50c-4a8e-b22c-327f2e7b8ab5"
src="https://github.com/user-attachments/assets/7c19e0c8-8c50-4f8c-b966-f2a824eea4a0"
/>
- Disable Auto Watch when following another collaborator, with test
coverage for that behavior. We currently disable following when engaging
auto watch, and now we disable auto watch when following. They are
mutually exclusive and I think the feels correct.
- Refactored Auto Watch integration tests to use channels API instead of
room API.
- Improved test robustness by using assertions to identify
`SharedScreen` items by type and `peer_id` instead of tab title text.
- Fixed Auto Watch for returning channel participants by emitting
`RemoteVideoTracksChanged` when removing a participant with active video
tracks, with regression coverage for leave/rejoin/share.
Self-Review Checklist:
- [X] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [X] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [X] Tests cover the new/changed behavior
- [X] Performance impact has been considered and is acceptable
Closes
Release Notes:
- N/A
This PR adds a feature to automatically cycle through screen shares
during calls, designed for demo days or any call that has a lot of
screen share use.
This is a preliminary attempt behind a feature flag so we can dogfood
and iterate, or toss it out.
There's a new toggle next to the active channel name in the collab
panel: **Auto Watch Screens**.
https://github.com/user-attachments/assets/ae6eccec-7921-4c1f-8921-c8093631c705
This video demonstrates some cases:
Basic auto-watch
- Toggle on → automatically opens the next screen share that starts
- When the watched screen share ends, switches to the next available
share
Queuing
- Someone starts sharing while another share is active → doesn't
interrupt the current share
- When the current share ends, the queued share is picked up
automatically
Paused while sharing
- Auto-watch pauses when you start sharing your own screen, so other
shares don't pop up during your presentation
- When you stop sharing, auto-watch resumes and opens the next available
share
Multiple watchers
- Multiple people can have auto-watch enabled independently — they all
see the same transitions
Note that we don't manage the screenshares, livekit does, so this change
is entirely on the client. I think that's mostly fine, but there is a
chance 2 separate clients queues up a different person as the next
watched peer if they both engage screenshare around the same time,
depending on how it hits the clients, but it seems pretty edge case. We
can move the implementation to collab, but it will be more of a project,
and adding a secondary source alongside of livekit that could get out of
sync and have its own issues.
UI/UX needs work (@danilo-leal for suggestions)
Self-Review Checklist:
- [X] I've reviewed my own diff for quality, security, and reliability
- [X] Unsafe blocks (if any) have justifying comments
- [X] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [X] Tests cover the new/changed behavior
- [X] Performance impact has been considered and is acceptable
Closes #ISSUE
Release Notes:
- N/A
---------
Co-authored-by: Yara 🏳️⚧️ <11743287+yara-blue@users.noreply.github.com>
This PR makes Zed only have one worktree picker, as opposed to a flavor
of it in the title bar and another in the agent panel. It then moves it
to the title bar, making it always present, so that its trigger is
separate from the branch picker (which now contains only two views:
branches and stashes). For the worktree picker, I'm mostly favoring the
behavior we've introduced in the agent-panel-flavored version.
It also updates the title bar settings migration to use the JSON
`migrate_settings` helper instead of a shallow Tree-sitter rewrite, so
old `show_branch_icon = true` values are promoted to
`show_branch_status_icon = true` across root, platform, release-channel,
and profile settings scopes.
- [x] Move worktree creation logic to the `git_ui` crate to make this
more generic and less agent-specific
- [x] Double-check the remote use case and ensure nothing broke there
- [x] Improve the UX for the detached HEAD state; better invite people
to create a branch
- [x] Migrate `show_branch_icon = true` to `show_branch_status_icon =
true` across nested settings scopes
Suggested .rules additions
When migrating renamed settings keys that can appear in platform
overrides, release-channel overrides, or profiles, prefer the JSON
`migrations::migrate_settings` helper over shallow Tree-sitter key
rewrites unless tests explicitly cover every nested scope that can
contain the key.
Release Notes:
- Improved migration of the title bar branch status icon setting.
---------
Co-authored-by: Nathan Sobo <nathan@zed.dev>
Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
Co-authored-by: Ben Brandt <benjamin.j.brandt@gmail.com>
TODO:
- [x] merge main
- [x] nonshrinking `set_excerpts_for_path`
- [x] Test-drive potential problem areas in the app
- [x] prepare cloud side
- [x] test collaboration
- [ ] docstrings
- [ ] ???
## Context
### Background
Currently, a multibuffer consists of an arbitrary list of
anchor-delimited excerpts from individual buffers. Excerpt ranges for a
fixed buffer are permitted to overlap, and can appear in any order in
the multibuffer, possibly separated by excerpts from other buffers.
However, in practice all code that constructs multibuffers does so using
the APIs defined in the `path_key` submodule of the `multi_buffer` crate
(`set_excerpts_for_path` etc.) If you only use these APIs, the resulting
multibuffer will maintain the following invariants:
- All excerpts for the same buffer appear contiguously in the
multibuffer
- Excerpts for the same buffer cannot overlap
- Excerpts for the same buffer appear in order
- The placement of the excerpts for a specific buffer in the multibuffer
are determined by the `PathKey` passed to `set_excerpts_for_path`. There
is exactly one `PathKey` per buffer in the multibuffer
### Purpose of this PR
This PR changes the multibuffer so that the invariants maintained by the
`path_key` APIs *always* hold. It's no longer possible to construct a
multibuffer with overlapping excerpts, etc. The APIs that permitted
this, like `insert_excerpts_with_ids_after`, have been removed in favor
of the `path_key` suite.
The main upshot of this is that given a `text::Anchor` and a
multibuffer, it's possible to efficiently figure out the unique excerpt
that includes that anchor, if any:
```
impl MultiBufferSnapshot {
fn buffer_anchor_to_anchor(&self, anchor: text::Anchor) -> Option<multi_buffer::Anchor>;
}
```
And in the other direction, given a `multi_buffer::Anchor`, we can look
at its `text::Anchor` to locate the excerpt that contains it. That means
we don't need an `ExcerptId` to create or resolve
`multi_buffer::Anchor`, and in fact we can delete `ExcerptId` entirely,
so that excerpts no longer have any identity outside their
`Range<text::Anchor>`.
There are a large number of changes to `editor` and other downstream
crates as a result of removing `ExcerptId` and multibuffer APIs that
assumed it.
### Other changes
There are some other improvements that are not immediate consequences of
that big change, but helped make it smoother. Notably:
- The `buffer_id` field of `text::Anchor` is no longer optional.
`text::Anchor::{MIN, MAX}` have been removed in favor of
`min_for_buffer`, etc.
- `multi_buffer::Anchor` is now a three-variant enum (inlined slightly):
```
enum Anchor {
Min,
Excerpt {
text_anchor: text::Anchor,
path_key_index: PathKeyIndex,
diff_base_anchor: Option<text::Anchor>,
},
Max,
}
```
That means it's no longer possible to unconditionally access the
`text_anchor` field, which is good because most of the places that were
doing that were buggy for min/max! Instead, we have a new API that
correctly resolves min/max to the start of the first excerpt or the end
of the last excerpt:
```
impl MultiBufferSnapshot {
fn anchor_to_buffer_anchor(&self, anchor: multi_buffer::Anchor) -> Option<text::Anchor>;
}
```
- `MultiBufferExcerpt` has been removed in favor of a new
`map_excerpt_ranges` API directly on `MultiBufferSnapshot`.
## Self-Review Checklist
<!-- Check before requesting review: -->
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Release Notes:
- N/A
---------
Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Conrad <conrad@zed.dev>
## Context
This fixes a participant location out of sync bug when the active
workspace changes in a multi workspace. It wouldn't update the project
id, which breaks following.
## Self-Review Checklist
<!-- Check before requesting review: -->
- [x] I've reviewed my own diff for quality, security, and reliability
- [x] Unsafe blocks (if any) have justifying comments
- [x] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [x] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Release Notes:
- N/A
Release Notes:
- Fixed 1-2 seconds of audio silence when unmuting for the first time
during a call with Bluetooth headphones when `mute_on_join` is enabled.
This will help with test times (in some cases), as nextest cannot figure
out whether a given rdep is actually an alive edge of the build graph
Closes #ISSUE
Before you mark this PR as ready for review, make sure that you have:
- [ ] Added a solid test coverage and/or screenshots from doing manual
testing
- [ ] Done a self-review taking into account security and performance
aspects
- [ ] Aligned any UI changes with the [UI
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
Release Notes:
- N/A
We've been considering removing workspace-hack for a couple reasons:
- Lukas ran into a situation where its build script seemed to be causing
spurious rebuilds. This seems more likely to be a cargo bug than an
issue with workspace-hack itself (given that it has an empty build
script), but we don't necessarily want to take the time to hunt that
down right now.
- Marshall mentioned hakari interacts poorly with automated crate
updates (in our case provided by rennovate) because you'd need to have
`cargo hakari generate && cargo hakari manage-deps` after their changes
and we prefer to not have actions that make commits.
Currently removing workspace-hack causes our workspace to grow from
~1700 to ~2000 crates being built (depending on platform), which is
mainly a problem when you're building the whole workspace or running
tests across the the normal and remote binaries (which is where
feature-unification nets us the most sharing). It doesn't impact
incremental times noticeably when you're just iterating on `-p zed`, and
we'll hopefully get these savings back in the future when
rust-lang/cargo#14774 (which re-implements the functionality of hakari)
is finished.
Release Notes:
- N/A
Before this change the active theme and icon theme were retrofitted onto
the ThemeSettings.
Now they're in their own new global (GlobalTheme::theme(cx) and
GlobalTheme::icon_theme(cx))
This lets us remove cx from the settings traits, and tidy up a few other
things along the way.
Release Notes:
- N/A
Release Notes:
- collab: A distinct sound effect is now used for when a guest joins a
call.
- collab: Fixed the "joined" sound being excessively loud when joining a
call that already has many participants.
---------
Co-authored-by: David Kleingeld <davidsk@zed.dev>
Closes https://github.com/zed-industries/zed/issues/38690Closes#37353
### Background
On Windows, paths are normally separated by `\`, unlike mac and linux
where they are separated by `/`. When editing code in a project that
uses a different path style than your local system (e.g. remoting from
Windows to Linux, using WSL, and collaboration between windows and unix
users), the correct separator for a path may differ from the "native"
separator.
Previously, to work around this, Zed converted paths' separators in
numerous places. This was applied to both absolute and relative paths,
leading to incorrect conversions in some cases.
### Solution
Many code paths in Zed use paths that are *relative* to either a
worktree root or a git repository. This PR introduces a dedicated type
for these paths called `RelPath`, which stores the path in the same way
regardless of host platform, and offers `Path`-like manipulation APIs.
RelPath supports *displaying* the path using either separator, so that
we can display paths in a style that is determined at runtime based on
the current project.
The representation of absolute paths is left untouched, for now.
Absolute paths are different from relative paths because (except in
contexts where we know that the path refers to the local filesystem)
they should generally be treated as opaque strings. Currently we use a
mix of types for these paths (std::path::Path, String, SanitizedPath).
Release Notes:
- N/A
---------
Co-authored-by: Cole Miller <cole@zed.dev>
Co-authored-by: Piotr Osiewicz <24362066+osiewicz@users.noreply.github.com>
Co-authored-by: Peter Tripp <petertripp@gmail.com>
Co-authored-by: Smit Barmase <heysmitbarmase@gmail.com>
Co-authored-by: Lukas Wirth <me@lukaswirth.dev>
When we refactored settings to not pass JSON blobs around, we ended up
needing
to write *a lot* of code that just merged things (like json merge used
to do).
Use a derive macro to prevent typos in this logic.
Release Notes:
- N/A
Co-Authored-By: Ben K <ben@zed.dev>
Co-Authored-By: Anthony <anthony@zed.dev>
Co-Authored-By: Mikayla <mikayla@zed.dev>
Release Notes:
- settings: Major internal changes to settings. The primary user-facing
effect is that some settings which did not make sense in project
settings files are no-longer read from there. (For example the inline
blame settings)
---------
Co-authored-by: Ben Kunkle <ben@zed.dev>
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
Co-authored-by: Anthony <anthony@zed.dev>
serde 1.0.221 introduced serde_core into the build graph, which should
render explicitly depending on serde_derive for faster build times an
obsolote method.
Besides, I'm not even sure if that worked for us. My hunch is that at
least one of our deps would have `serde` with derive feature enabled..
and then, most of the crates using `serde_derive` explicitly were also
depending on gpui, which depended on `serde`.. thus, we wouldn't have
gained anything from explicit dep on `serde_derive`
Release Notes:
- N/A
This PR separates out the associated constant `KEY` from the `Settings`
trait into a new trait `SettingsKey`. This allows for the key trait to
be derived using attributes to specify the path so that the new
`SettingsUi` derive macro can use the same attributes to determine top
level settings paths thereby removing the need to duplicate the path in
both `Settings::KEY` and `#[settings_ui(path = "...")]`
Co-authored-by: Ben Kunkle <ben@zed.dev>
Release Notes:
- N/A
---------
Co-authored-by: Ben Kunkle <ben@zed.dev>
Closes #ISSUE
Initially, the `SettingsUi` trait was tied to `Settings`, however, given
that the `Settings::FileContent` type (which may be the same as the type
that implements `Settings`) will be the type that more directly maps to
the JSON structure (and therefore have the documentation, correct field
names (or `serde` rename attributes), etc) it makes more sense to have
the deriving of `SettingsUi` occur on the `FileContent` type rather than
the `Settings` type.
In order for this to work a relatively important change had to be made
to the derive macro, that being that it now "unwraps" options into their
inner type, so a field with type `Option<Foo>` where `Foo: SettingsUi`
will treat the field as if it were just `Foo`, expecting there to be a
default set in `default.json`. This imposes some restrictions on what
`Settings::FileContent` can be as seen in 1e19398 where `FileContent`
itself can't be optional without manually implementing `SettingsUi`, as
well as introducing some risk that if the `FileContent` type has
`serde(default)`, the default value will override the default value from
`default.json` in the UI even though it may differ (but it should!).
A future PR should probably replace the other settings with `FileContent
= Option<T>` (all of which currently have `T == bool`) with wrapper
structs and have `KEY = None` so the further niceties
`derive(SettingsUi)` will provide such as path renaming, custom UI, auto
naming and doc comment extraction can be used.
Release Notes:
- N/A *or* Added/Fixed/Improved ...
## Goal
This PR creates the initial settings ui structure with the primary goal
of making a settings UI that is
- Comprehensive: All settings are available through the UI
- Correct: Easy to understand the underlying JSON file from the UI
- Intuitive
- Easy to implement per setting so that UI is not a hindrance to future
settings changes
### Structure
The overall structure is settings layer -> data layer -> ui layer.
The settings layer is the pre-existing settings definitions, that
implement the `Settings` trait. The data layer is constructed from
settings primarily through the `SettingsUi` trait, and it's associated
derive macro. The data layer tracks the grouping of the settings, the
json path of the settings, and a data representation of how to render
the controls for the setting in the UI, that is either a marker value
for the component to use (avoiding a dependency on the `ui` crate) or a
custom render function.
Abstracting the data layer from the ui layer allows crates depending on
`settings` to implement their own UI without having to add additional UI
dependencies, thus avoiding circular dependencies. In cases where custom
UI is desired, and a creating a custom render function in the same crate
is infeasible due to circular dependencies, the current solution is to
implement a marker for the component in the `settings` crate, and then
handle the rendering of that component in `settings_ui`.
### Foundation
This PR creates a macro and a trait both called `SettingsUi`. The
`SettingsUi` trait is added as a new trait bound on the `Settings`
trait, this allows the type system to guarantee that all settings
implement UI functionality. The macro is used to derived the trait for
most types, and can be modified through attributes for unique cases as
well.
A derive-macro is used to generate the settings UI trait impl, allowing
it the UI generation to be generated from the static information in our
code base (`default.json`, Struct/Enum names, field names, `serde`
attributes, etc). This allows the UI to be auto-generated for the most
part, and ensures consistency across the UI.
#### Immediate Follow ups
- Add a new `SettingsPath` trait that will be a trait bound on
`SettingsUi` and `Settings`
- This trait will replace the `Settings::key` value to enable
`SettingsUi` to infer the json path of it's derived type
- Figure out how to render `Option<T> where T: SettingsUi` correctly
- Handle `serde` attributes in the `SettingsUi` proc macro to correctly
get json path from a type's field and identity
Release Notes:
- N/A
---------
Co-authored-by: Ben Kunkle <ben@zed.dev>
This is a pure refactor that consolidates all SSH remoting logic such
that it should be straightforward to add another transport to the
remoting system.
Release Notes:
- N/A
---------
Co-authored-by: Mikayla Maki <mikayla.c.maki@gmail.com>
Instead of selecting a screen to share arbitrarily, we'll now allow user
to select the screen to share. Note that sharing multiple screens at the
time is still not supported (though prolly not too far-fetched).
Related to #4666
Release Notes:
- Added screen selector dropdown to screen share button
---------
Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Co-authored-by: Cole Miller <cole@zed.dev>
Closes#30017
* While generating the settings JSON schema, defaults all schema
definitions to reject unknown fields via `additionalProperties: false`.
* Uses `unevaluatedProperties: false` at the top level to check fields
that remain after the settings field names + release stage override
field names.
* Changes json schema version from `draft07` to `draft_2019_09` to have
support for `unevaluatedProperties`.
Release Notes:
- Added warnings for unknown fields when editing `settings.json`.
https://github.com/zed-industries/zed/issues/30972 brought up another
case where our context is not enough to track the actual source of the
issue: we get a general top-level error without inner error.
The reason for this was `.ok_or_else(|| anyhow!("failed to read HEAD
SHA"))?; ` on the top level.
The PR finally reworks the way we use anyhow to reduce such issues (or
at least make it simpler to bubble them up later in a fix).
On top of that, uses a few more anyhow methods for better readability.
* `.ok_or_else(|| anyhow!("..."))`, `map_err` and other similar error
conversion/option reporting cases are replaced with `context` and
`with_context` calls
* in addition to that, various `anyhow!("failed to do ...")` are
stripped with `.context("Doing ...")` messages instead to remove the
parasitic `failed to` text
* `anyhow::ensure!` is used instead of `if ... { return Err(...); }`
calls
* `anyhow::bail!` is used instead of `return Err(anyhow!(...));`
Release Notes:
- N/A
Kirill Bulatov