Skip to content

Fix zero-width grapheme attachment during pending wrap - #12581

Merged
mitchellh merged 1 commit into
ghostty-org:mainfrom
noib3:zero-width-attachments
May 4, 2026
Merged

mitchellh merged 1 commit into
ghostty-org:mainfrom
noib3:zero-width-attachments

Conversation

@noib3

@noib3 noib3 commented May 4, 2026

Copy link
Copy Markdown
Contributor

This PR fixes an issue where a zero-width combining mark could attach to the wrong cell when the preceding character was written in the final column and the cursor had a pending wrap.

The test I added used to fail before the fix, but it passes now.

@noib3
noib3 requested a review from a team as a code owner May 4, 2026 20:59

@mitchellh mitchellh left a comment

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.

This makes sense, we should add a test to Terminal.zig too though that's the more appropriate place for this since the C tests are only run conditionally. You can keep that C test too though.

@mitchellh mitchellh added this to the 1.3.2 milestone May 4, 2026
@noib3
noib3 force-pushed the zero-width-attachments branch from 15cf1d7 to 04e8824 Compare May 4, 2026 21:12
@noib3

noib3 commented May 4, 2026

Copy link
Copy Markdown
Contributor Author

Added an equivalent test to Terminal.zig.

Not sure what the policy is wrt to test redundancy. I can remove the test in c/terminal.zig since that code path is now already covered.

This PR fixes an issue where a zero-width combining mark could attach to
the wrong cell when the preceding character was written in the final
column and the cursor had a pending wrap.
@noib3
noib3 force-pushed the zero-width-attachments branch from 04e8824 to c20fcfa Compare May 4, 2026 21:16

@mitchellh mitchellh left a comment

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.

Approved pending CI.

General note: print is one of our most performance sensitive pieces of code, but this is specifically in the width == 0 branch which is not common, so adding more logic here is fine.

Not sure what the policy is wrt to test redundancy. I can remove the test in c/terminal.zig since that code path is now already covered.

Its fine, I don't mind duplicate tests that test at different layers.

@mitchellh
mitchellh merged commit 563b085 into ghostty-org:main May 4, 2026
89 checks passed
@noib3
noib3 deleted the zero-width-attachments branch May 4, 2026 21:38
noib3 added a commit to noib3/neovim that referenced this pull request May 4, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request May 5, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request May 12, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request May 13, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request May 18, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request May 22, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request Jun 4, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request Jul 12, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request Jul 23, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
noib3 added a commit to noib3/neovim that referenced this pull request Jul 25, 2026
We need the fix I implemented in
ghostty-org/ghostty#12581 for our tests to pass.
@mitchellh mitchellh modified the milestones: 1.3.2, 1.4.0 Oct 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants