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
Fixes a bug in the split diff spacer calculation when a patch group
starts mid-row, sometimes causing extra spacers to be inserted.
`spacer_blocks` already explicitly handles the case where `first_point`
isn't at the start of `edit_for_first_point.old`, but the `while let
Some(source_point) = source_points.next()` loop that follows implicitly
assumes that `source_point` is at the start of `current_range`, which in
turn seems to be based on the assumption that `current_range` starts at
the beginning of a row. As it turns out, `current_range` isn't
guaranteed to start at the beginning of a row, which can sometimes lead
to incorrect spacer blocks being inserted.
This addresses that by moving the existing `if
edit_for_first_point.old.start < first_point` logic into the loop body
as `if current_edit.old.start < current_boundary` in order to handle any
non-row-aligned patch groups, not just the first one.
Here's an example of how this bug could manifest:
https://github.com/user-attachments/assets/1d3a5b4c-e4ad-4d87-804b-c4390d25f408
After:
https://github.com/user-attachments/assets/b15acc62-33fe-4154-82e5-5cdf1806ffa7
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:
- Fixed incorrect spacer blocks sometimes appearing in the split diff
view when editing the file.
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)
- [ ] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Release Notes:
- Fixed a panic when using the split diff view.
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)
- [ ] Tests cover the new/changed behavior
- [x] Performance impact has been considered and is acceptable
Closes#50212
There are two unreasonable coupling account for this issue, the coupling
of `use_tree_sitter` with `languge_aware` in
https://github.com/zed-industries/zed/blob/7892b932795911516f26f3c1c1c72249ed181ba8/crates/editor/src/element.rs#L3820-L3822
and the coupling of `language_aware` with `diagnostics` in
https://github.com/zed-industries/zed/blob/7892b932795911516f26f3c1c1c72249ed181ba8/crates/language/src/buffer.rs#L3736-L3746
Because of these couplings, when the editor stops using Tree-sitter
highlighting when `"semantic_tokens"` set to `"full"`, it also
accidentally stops fetching diagnostic information. This is why error
and warning underlines disappear.
I’ve fixed this by adding a separate `use_tree_sitter` parameter to
`highlighted_chunks`. This way, we can keep `language_aware` true to get
the diagnostic data we need, but still decide whether or not to apply
Tree-sitter highlights. I chose to fix this at the `highlighted_chunks`
level because I’m worried that changing the logic in the deeper layers
of the DisplayMap or Buffer might have too many side effects that are
hard to predict. This approach feels like a safer way to solve the
problem.
Release Notes:
- Fixed a bug where diagnostic underlines disappeared when
"semantic_tokens" set to "full"
---------
Co-authored-by: Kirill Bulatov <kirill@zed.dev>
It's possible to create a multibuffer anchor that points into a specific
excerpted buffer (so not min/max), but whose main buffer `text::Anchor`
isn't contained in any of the excerpts for that buffer. When summarizing
such an anchor, we map it to the multibuffer position of the start of
the next excerpt after where the anchor "should" appear. Or at least,
that's the intention, but it turned out we had some bugs in
`summary_for_anchor` and `summaries_for_anchors` that caused them to
return bizarre summaries for these anchors. This PR fixes that and also
updates `test_random_multibuffer` to actually test
`MultiBufferSnapshot::summary_for_anchor` against a reference
implementation, including for out-of-bounds anchors.
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
---------
Co-authored-by: Anthony <anthony@zed.dev>
Co-authored-by: Max <max@zed.dev>
Co-authored-by: Anthony Eid <hello@anthonyeid.me>
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>
Self-Review Checklist:
- [ ] I've reviewed my own diff for quality, security, and reliability
- [ ] Unsafe blocks (if any) have justifying comments
- [ ] The content is consistent with the [UI/UX
checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist)
- [ ] Tests cover the new/changed behavior
- [ ] Performance impact has been considered and is acceptable
Closes #ISSUE
Release Notes:
- N/A
In `BlockMap::sync`, blocks within an edited region are sorted and
processed sequentially. Each block placement computes
`rows_before_block` by subtracting `new_transforms.summary().input_rows`
from the block's target position. The `Near`/`Below` cases have a guard
that skips the block if the target is already behind the current
progress, but `Above` and `Replace` were missing this guard.
When a `Replace` block (tie_break 0) is processed before an `Above`
block (tie_break 1) at the same or overlapping position, the `Replace`
block consumes multiple input rows, advancing `input_rows` past the
`Above` block's position. The subsequent `position - input_rows`
subtraction underflows on `u32`, producing a huge `RowDelta` that wraps
`wrap_row_end` past `wrap_row_start`, creating an inverted range that
propagates through the display map layers and panics as `begin <= end
(47 <= 0)` in a rope chunk slice.
Add underflow guards to `Above` and `Replace`, matching the existing
pattern in `Near`/`Below`.
Release Notes:
- Fixed a source of underflowing subtractions causing spurious panics
Release Notes:
- Fixed multiple language servers applying to the same buffer
overwriting each others inlay hints
- Fixed multiple language servers applying to the same multibuffer
discarding each others inlay hints
- Fixed a bug that caused some inlay hints to sometimes duplicate
Paving the way to remove `ExcerptId`. Done in this PR:
- Unshipped the stack trace view
- Get rid of `push_excerpts`
- Get rid of some callers of `remove_excerpts`
We still need to remove some calls to `remove_excerpts` and other APIs,
especially in `randomly_edit_excerpts` and collaboration.
Release Notes:
- The stack trace multibuffer view has been removed.
---------
Co-authored-by: Conrad Irwin <conrad.irwin@gmail.com>
These tend to be very often mutated while we don't really need the
subtree sharing of TreeMaps, so replacing them here is generally
cheaper.
Release Notes:
- N/A *or* Added/Fixed/Improved ...
Co-authored by: John Tur <jtur@zed.dev>
fixes ZED-57B
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:
- Fixed a (rare) panic when a fold was created immediately following a
tab character
For inverted diff APIs like `hunks_intersecting_base_text_range`, we
need a snapshot of the main buffer to use as context to compare hunk
anchors, convert anchors to points, etc. Previously, we were always
using a snapshot of the main buffer at the time when the diff was
calculated. This worked well for the hunks of the primary `BufferDiff`,
but it's not valid when the diff also has a secondary set of hunks,
because those hunks are recalculated on their own schedule--so it's
possible for the hunks of the secondary diff to contain anchors that are
newer than the original buffer snapshot that was taken at the time the
primary diff hunks were last calculated. This caused a panic when using
these anchors with the original buffer snapshot.
This PR fixes the issue by using the same approach for inverted diffs as
for non-inverted diffs at the multibuffer level: we take a snapshot of
the main buffer at the time when we snapshot the diff state (including
the diff itself), guaranteeing that that snapshot will be new enough to
resolve all the anchors included in both the primary diff and the
secondary diff.
Closes ZED-54J
Release Notes:
- Fixed a panic that could occur when using the branch diff in split
view mode.
---------
Co-authored-by: Antonio Scandurra <me@as-cii.com>
Every yield will cause the background task to get rescheduled causing
additional thread/context switching, so doing so for every wrapped row
is a bit excessive
Release Notes:
- N/A *or* Added/Fixed/Improved ...
Replaces a bunch of `impl FnMut` parameters with `&mut dyn FnMut` for
functions where this is the sole generic parameter.
Release Notes:
- N/A *or* Added/Fixed/Improved ...
Currently, the next chunk is generated by traversing all characters of
the current folded chunk until a tab is found. Since we already have
bitmasks for characters and tabs, we can also propagate the newlines
bitmap from the rope and use it to make this computation O(1) in all
cases.
I haven’t run benchmarks yet.
Release Notes:
- N/A
- Defer syncing block maps from `set_companion`, eliminating some
redundant recomputations
- Emit one large multibuffer edit from `set_show_deleted_hunks` instead
of many small edits, to avoid bad case for block map
This cuts hangs roughly in half when toggling between views in a large
diff (1000 commits from the chromium repository).
<!-- 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:
- Improved performance with large diffs when toggling between diff
views.
---------
Co-authored-by: Jakub Konka <kubkon@jakubkonka.com>
Co-authored-by: Lukas Wirth <lukas@zed.dev>
We had a call to `BlockMap::unfold_intersecting` that ended up
recomputing the entire block map for the RHS _without_ spacers, only to
throw it away in favor of the version with spacers a few lines down. Now
we only sync each block map once in `set_companion`.
Release Notes:
- Improved performance when toggling from the unified diff to the split
diff.
Co-authored-by: Jakub <jakub@zed.dev>
Co-authored-by: Cameron McLoughlin <cameron.studdstreet@gmail.com>
- [x] Tests or screenshots needed?
- [ ] Code Reviewed
- [x] Manual QA
Release Notes:
- Fixed misalignment of lines in the split diff when using inlay hints.
Release Notes:
- Fixed inlay hints being rendered as new inserted words in word based
diff highlighting
---------
Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
- Ensure that both sides are passed the appropriate companion data to
preserve spacers when syncing
- Remove companion handling in codepaths related to range folding, since
this isn't supported in the side-by-side diff
- Move handling of buffer folding into the block map
- Rework `set_companion` to handle both `DisplayMap`s at once
- DRY some code around block map syncing in the `DisplayMap`
TODO:
- [x] diagnose and fix issue that causes balancing blocks not to render
properly when they are adjacent to spacers (e.g. merge conflict buttons)
- [x] clear balancing blocks when clearing companion
- [x] additional tests: interaction between spacers and balancing
blocks, resizing
Release Notes:
- N/A
Release Notes:
- Added handling of custom blocks in the RHS editor by creating matching
dummy custom blocks rendered as spacer blocks in the LHS editor when in
split view.
Follow-up of https://github.com/zed-industries/zed/pull/46356
* better selection in highlights panel to show the color of the active
item
* no `DiffbaselessAnchor` entity to cause flickering and highlighting
issues
* react on theme and theme settings change and refresh semantic tokens
* do not eagerly flicker when refreshing the tokens
* do not show semantic tokens in the editor menu for non-applicable
buffers
Release Notes:
- N/A
Part of #7450
Big thanks to @macmv for pushing this forwards so much!
Rebased version of https://github.com/zed-industries/zed/pull/39539 as
working on an in-org branch simplifies a lot of things for us)
Release Notes:
- Added LSP semantic tokens highlighting support
---------
Co-authored-by: Neil Macneale V <neil.macneale.v@gmail.com>
Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
`spacer_blocks` is an ideal place to drop `WrapPointCursor` since a
large part of what it does it to convert an increasing sequence of
`MultiBufferPoint` into `WrapPoint`.
Release Notes:
- N/A
The side-by-side diff heavily relies on a primitive from `buffer_diff`
that converts a point on one side of the diff to a range of points on
the other side. The way this primitive is set up on main is pretty
naive--every time we call `points_to_base_text_points` (or
`base_text_points_to_points`), we need to iterate over all hunks in the
diff. That's particularly bad for the case of constructing a new
side-by-side diff starting from a multibuffer, because we call those
APIs once per excerpt, and the number of excerpts is ~equal to the
number of hunks.
This PR changes the point translation APIs exposed by `buffer_diff` to
make it easier to use them efficiently in `editor`. The new shape is a
pair of functions that return a patch that can be used to translate from
the main buffer to the base text or vice versa. When syncing edits
through the block map that touch several excerpts for the same buffer,
we can reuse this patch for excerpts after the first--so when building a
new side-by-side diff, we'll iterate over each hunk just once.
The shape of the new APIs also sets us up to scale down to cases like
editing on the right-hand side of the diff: we can pass in a point range
and give them permission to return an approximate patch that's only
guaranteed to give the correct results when used with points in that
range. For edits that only affect one excerpt, and given how the project
diff is set up, that should allow us to skip iterating over most of the
hunks in a buffer.
Release Notes:
- N/A
---------
Co-authored-by: cameron <cameron.studdstreet@gmail.com>
Release Notes:
- N/A
--
I think it's possible to clean this up further but will withhold until
we land bits necessary to correctly align split view in presence of
custom blocks.
When committing, it was possible for the left-hand side multibuffer to
get the updated base text (via the `buffer_changed_since_sync`
mechanism) without updating its diff snapshot for that buffer (via the
diff subscription), causing a mismatch between that multibuffer's diff
state and its buffer state. The fix is to ensure for inverted diffs that
we always update pull an updated diff snapshot as part of
`sync_from_buffer_changes`.
This also removes some code that we added in #44838 to sync the
left-hand side multibuffer when edits on the right-hand side invalided
diff hunks. Instead, the left-hand side will just sync the next time the
diff recalculates when this happens, and will always consider hunks from
the last diff calculation as valid--so there will be a short window
where the diff transforms and `diff_hunks_in_range` don't match between
the two sides. That's okay because we don't rely on this in the display
map--the code that translates positions between the two sides accesses
the diff's `InternalDiffHunk`s directly rather than going through a
multibuffer API.
Release Notes:
- N/A
This PR implements a UI for the side-by-side diff, using blocks to align
the two sides and adding a coherent `SplitEditorElement`.
Release Notes:
- N/A
---------
Co-authored-by: cameron <cameron.studdstreet@gmail.com>
Co-authored-by: Anthony Eid <hello@anthonyeid.me>
Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
Yara 🏳️⚧️