Sitelet https://github.com/microsoft/edit/pull/970
Skip to content

Don't require grapheme boundaries - #970

Merged
Leonard Hecker (lhecker) merged 2 commits into
mainfrom
dev/lhecker/grapheme-boundaries
Oct 1, 2026
Merged

Leonard Hecker (lhecker) merged 2 commits into
mainfrom
dev/lhecker/grapheme-boundaries

Conversation

@lhecker

Copy link
Copy Markdown
Member

When inserting a lone CR in front of a LF in a document,
it joins to form a single grapheme cluster (CRLF) but
the gap buffer will split the grapheme into two chunks.
This commit fixes navigation across such chunks.

@lhecker
Leonard Hecker (lhecker) enabled auto-merge (squash) October 1, 2026 16:25
Comment on lines +107 to +108
let rest =
if rest.is_empty() { self.doc.read_forward(self.offset + 1) } else { rest };

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The fix.

self.offset += self.chunk.len();
self.chunk = self.doc.read_forward(self.offset);
self.chunk_off = 0;
self.read();

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Remove duplicate code.

// We can rely on the fact that the document does not split graphemes across chunks.
// = If there's a newline it's wholly contained in this chunk.
if self.chunk_off > 0 && self.chunk[self.chunk_off - 1] == b'\n' {
if self.chunk.get(self.chunk_off.wrapping_sub(1)) == Some(&b'\n') {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The compiler can't know that self.chunk_off < self.chunk.len() so this conditional .get() access helps it avoid panics.

@lhecker
Leonard Hecker (lhecker) merged commit c04475c into main Oct 1, 2026
6 checks passed
@lhecker
Leonard Hecker (lhecker) deleted the dev/lhecker/grapheme-boundaries branch October 1, 2026 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants