Skip to content

editor: Fix columnar selection alignment on rows with multi-byte chars - #57097

Merged
tomhoule merged 3 commits into
zed-industries:mainfrom
Dnreikronos:fix/columnar-selection-diacritic-56748
Jun 12, 2026
Merged

editor: Fix columnar selection alignment on rows with multi-byte chars#57097
tomhoule merged 3 commits into
zed-industries:mainfrom
Dnreikronos:fix/columnar-selection-diacritic-56748

Conversation

@Dnreikronos

@Dnreikronos Dnreikronos commented May 18, 2026

Copy link
Copy Markdown
Contributor

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
  • Tests cover the new/changed behavior
  • Performance impact has been considered and is acceptable

Closes #56748

Summary

Block / column selection (Shift+Ctrl+drag on Linux, Shift+Cmd+drag on macOS) anchored its rectangle on a single byte column applied to every row in the dragged range. On a row whose text contained a multi-byte character before that column (for example ã), the same byte offset mapped to a different visual x, so the per-row cursor drifted off the column.

select_columns now anchors the rectangle in x pixels via x_for_display_point and resolves each row's byte column with display_column_for_x — the same pair of helpers already used by vertical cursor motion and Vim VisualBlock.

Before / After

Before — cursor on the ã row drifts one column to the left:

image

After — rectangle stays visually aligned across rows:

image

Test plan

  • cargo test -p editor test_columnar_selection_with_multibyte_chars (new regression test covers the buggy case past the ã byte boundary, plus a control case where old and new logic agree).
  • Manual: Shift+Ctrl+drag down a five-line buffer where row 3 contains ã — selection rectangle stays aligned (see screenshots).
  • Manual: Shift+Alt+drag (FromSelection branch) over the same buffer — same result.
  • Manual: drag past EOL on the ã row — each row extends to its own EOL with no panic.

Release Notes:

  • Fixed misalignment of column selection on rows that contain multi-byte characters

Block-selection (Shift+Ctrl+drag / Shift+Cmd+drag) anchored its
rectangle on a single byte column applied to every row. On rows whose
text contained multi-byte characters before that column (e.g. `ã`), the
same byte offset mapped to a different visual x, drifting the per-row
cursor off the column.

Anchor the rectangle in x pixels using x_for_display_point and resolve
each row's byte columns with display_column_for_x — the same approach
used by vertical cursor motion and Vim VisualBlock.

Closes zed-industries#56748
@cla-bot cla-bot Bot added the cla-signed The user has signed the Contributor License Agreement label May 18, 2026
@MrSubidubi MrSubidubi added the area:editor Feedback for code editing, formatting, editor iterations, etc label May 19, 2026
@tomhoule tomhoule self-assigned this Jun 5, 2026
&text_layout_details,
);
let start_x = tail_x.min(head_x);
let end_x = tail_x.max(head_x);

@tomhoule tomhoule Jun 5, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am wondering if this is the expected behaviour for users who use variable width fonts (also applies to mixed language text where some of it may be variable width). I know a few.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, as I remember, this is intentional. it's basically the same thing Zed already does in a couple other spots:

  • vertical cursor movement (Up/Down) keeps the cursor at the same visual x across rows of different widths, via SelectionGoal::HorizontalPosition + display_column_for_x in editor/src/movement.rs.
  • Vim's Ctrl-V VisualBlock also builds its rectangle from a pixel range, resolving each row with closest_index_for_x in vim/src/visual.rs.

a columnar selection is kinda just a visual rectangle, so anchoring in x is what keeps the left/right edges actually vertical on screen for variable-width fonts and mixed-width text like CJK. the old byte-column anchoring drew a ragged selection in exactly those cases, which is the same root cause behind #56748. so imo this just brings mouse columnar selection in line with the two cases above instead of leaving it as the odd one out.

idk, if you'd rather have a column-count model for variable-width fonts i'm open to it, but that'd diverge from how vertical motion and VisualBlock already behave. lgtm as is though, lmk what you think.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right! Reviewing the implementation now.

…n-diacritic-56748

# Conflicts:
#	crates/editor/src/editor_tests.rs
@Dnreikronos
Dnreikronos force-pushed the fix/columnar-selection-diacritic-56748 branch from 39dfdae to 4f18be1 Compare June 10, 2026 15:33
When the cursor was past the end of any of the lines in the selected block, the selected area would clip to the last column of the shortest line, which does not line up with the expectation for a rectangular selection.
@tomhoule
tomhoule force-pushed the fix/columnar-selection-diacritic-56748 branch from 4f18be1 to d462b8b Compare June 12, 2026 09:20
@tomhoule

Copy link
Copy Markdown
Contributor

I noticed the behaviour was not completely expected. When the cursor was past the end of any of the lines in the selected block, the selected area would clip to the last column of the shortest line, which does not line up with the expectation for a rectangular selection, so I pushed some tweaks.

Here's current main:

Screen.Recording.2026-06-12.at.11.05.45.mov

This branch before:

Screen.Recording.2026-06-12.at.11.08.02.mov

This branch after:

Screen.Recording.2026-06-12.at.11.10.31.mov

Notice there's still some odd behaviour when the cursor is past the end of the text area.

@tomhoule

Copy link
Copy Markdown
Contributor

Notice there's still some odd behaviour when the cursor is past the end of the text area.

I looked into fixing that, and it's not completely trivial, so best left to another PR.

This branch is already a big improvement, so let's merge it. Thank you for the contribution!

@tomhoule
tomhoule added this pull request to the merge queue Jun 12, 2026
Merged via the queue into zed-industries:main with commit 992f395 Jun 12, 2026
32 checks passed
This was referenced Jun 18, 2026
jonx pushed a commit to jonx/zed-aros that referenced this pull request Jul 17, 2026
zed-industries#57097)

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 zed-industries#56748

## Summary

Block / column selection (Shift+Ctrl+drag on Linux, Shift+Cmd+drag on
macOS) anchored its rectangle on a single byte column applied to every
row in the dragged range. On a row whose text contained a multi-byte
character before that column (for example `ã`), the same byte offset
mapped to a different visual `x`, so the per-row cursor drifted off the
column.

`select_columns` now anchors the rectangle in `x` pixels via
`x_for_display_point` and resolves each row's byte column with
`display_column_for_x` — the same pair of helpers already used by
vertical cursor motion and Vim VisualBlock.

## Before / After

**Before** — cursor on the `ã` row drifts one column to the left:

<img width="1068" height="366" alt="image"
src="https://github.com/user-attachments/assets/0559ef89-852e-40f3-9d43-f5f44277f99e"
/>


**After** — rectangle stays visually aligned across rows:

<img width="866" height="408" alt="image"
src="https://github.com/user-attachments/assets/bce13c1a-1ddb-4fd6-8216-2c0e902f6668"
/>


## Test plan

- `cargo test -p editor test_columnar_selection_with_multibyte_chars`
(new regression test covers the buggy case past the `ã` byte boundary,
plus a control case where old and new logic agree).
- Manual: Shift+Ctrl+drag down a five-line buffer where row 3 contains
`ã` — selection rectangle stays aligned (see screenshots).
- Manual: Shift+Alt+drag (`FromSelection` branch) over the same buffer —
same result.
- Manual: drag past EOL on the `ã` row — each row extends to its own EOL
with no panic.

Release Notes:

- Fixed misalignment of column selection on rows that contain multi-byte
characters

---------

Co-authored-by: Tom Houlé <tom@tomhoule.com>
jolutz pushed a commit to jolutz/zed that referenced this pull request Aug 8, 2026
zed-industries#57097)

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 zed-industries#56748

## Summary

Block / column selection (Shift+Ctrl+drag on Linux, Shift+Cmd+drag on
macOS) anchored its rectangle on a single byte column applied to every
row in the dragged range. On a row whose text contained a multi-byte
character before that column (for example `ã`), the same byte offset
mapped to a different visual `x`, so the per-row cursor drifted off the
column.

`select_columns` now anchors the rectangle in `x` pixels via
`x_for_display_point` and resolves each row's byte column with
`display_column_for_x` — the same pair of helpers already used by
vertical cursor motion and Vim VisualBlock.

## Before / After

**Before** — cursor on the `ã` row drifts one column to the left:

<img width="1068" height="366" alt="image"
src="https://github.com/user-attachments/assets/0559ef89-852e-40f3-9d43-f5f44277f99e"
/>


**After** — rectangle stays visually aligned across rows:

<img width="866" height="408" alt="image"
src="https://github.com/user-attachments/assets/bce13c1a-1ddb-4fd6-8216-2c0e902f6668"
/>


## Test plan

- `cargo test -p editor test_columnar_selection_with_multibyte_chars`
(new regression test covers the buggy case past the `ã` byte boundary,
plus a control case where old and new logic agree).
- Manual: Shift+Ctrl+drag down a five-line buffer where row 3 contains
`ã` — selection rectangle stays aligned (see screenshots).
- Manual: Shift+Alt+drag (`FromSelection` branch) over the same buffer —
same result.
- Manual: drag past EOL on the `ã` row — each row extends to its own EOL
with no panic.

Release Notes:

- Fixed misalignment of column selection on rows that contain multi-byte
characters

---------

Co-authored-by: Tom Houlé <tom@tomhoule.com>
InfyniteHeap pushed a commit to InfyniteHeap/zed that referenced this pull request Aug 14, 2026
…ed-industries#61997)

# Objective

Fixes zed-industries#60192
Closes zed-industries#62308

`editor: align selection` lines cursors up by their buffer column, and
that column counts bytes. If a multi-byte character sits before the
cursor, the byte column is larger than the position the cursor is
actually drawn at, so the row gets padded with the wrong number of
spaces.

The issue reports it with `←` (3 bytes) and `π` (2 bytes):

```
a ← 1  # one
bc ← π  # two
```

Put a cursor on each `#`, run the action, and the result is still
misaligned:

```
a ← 1    # one
bc ← π  # two
```

This is not the columnar selection bug fixed in zed-industries#57097. That one was
`select_columns` in `selection.rs`, where the output is a selection
range. This one is `align_selections` in `editor.rs`, where the output
is inserted spaces, so the same byte-column assumption was left behind
in a second place, and fixing it here needs a rounding step that the
first fix did not.

## Solution

Measure each cursor by its x offset in the laid-out display row
(`DisplaySnapshot::x_for_display_point`), take the target for a column
as the furthest x across the rows, then turn the difference into whole
spaces by dividing by the advance width of `' '`. The offset that
carries into later columns becomes an x offset instead of a column
count.

The display map has already expanded tabs by the time the row is laid
out, so a leading tab now counts as its expanded width instead of as a
single byte.

Two things I would look at first in review:

- The division rounds instead of truncating. The x offsets are built by
repeated float addition, so a gap that should be exactly three spaces
can arrive as 2.9999998, and truncating inserts two.
- The function returns early if the space advance is missing or zero.
Dividing by zero gives `inf`, which saturates to a huge `u32` and then
tries to allocate that many spaces.

I did not add any public items and did not touch `selection.rs`.

## Testing

`cargo test -p editor align` on Windows: 6 passed, 0 failed. That is the
new test plus the two existing `align_selections` tests, which I did not
change and which still pass.

`test_align_selections_with_multibyte_chars` covers the repro from the
issue, a second column whose offset has to carry past a multi-byte
character in the first, a leading tab, a non-BMP character, and a case
where multi-byte characters sit after the cursors and nothing should
move.

I also checked that the test catches the bug rather than just passing:
reverting the change in `editor.rs` and keeping the test makes it fail
on the repro, inserting four spaces where three are right. Putting the
change back makes it pass. The two older align tests pass either way,
since they are pure ASCII.

What I have not covered:

- Wide CJK characters, combining marks, and ZWJ clusters. These should
be right by construction, since the code measures advances rather than
counting characters, but I have no tests for them. The headless text
system behind `gpui::test` gives every BMP character the same advance,
so a test there would assert the test double's behavior rather than the
real renderer's.
- Proportional fonts. Aligning with inserted spaces cannot be exact when
glyph widths vary. The code rounds to the nearest whole space.
- Soft-wrapped rows. I measure x from the start of the wrapped row but
still group cursors by buffer row, so two cursors on one buffer row that
sit either side of a wrap boundary get measured from different origins,
and the carried offset crosses that boundary as if they shared one. The
old byte-column code did not have that particular failure. I left it
alone because fixing it is a different change, but I would rather flag
it than have you find it.
- I work on Windows and have no macOS machine. The arithmetic is
platform independent, so I do not expect a difference, but I have not
checked.

To try it: paste the two lines from the issue, put a cursor on each `#`
with `editor: select next`, then run `editor: align selection`. The two
`#` should line up.

## Self-Review Checklist:

- [ ] 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 `editor: align selections` misaligning rows and Vim `ctrl-d` /
`ctrl-u` / `ctrl-f` leaving the cursor behind on lines with multi-byte
characters or tabs.

---------

Co-authored-by: Kirill Bulatov <kirill@zed.dev>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:editor Feedback for code editing, formatting, editor iterations, etc cla-signed The user has signed the Contributor License Agreement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Column selection: wrong selection with diacritics

3 participants