Repository navigation
Typed gapbuffer indexing - #191
Merged
Merged
Conversation
sminez
force-pushed
the
typed-gapbuffer-indexing
branch
from
November 1, 2026 11:32
026dbc9 to
e484646
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a big one...
Mixing up character and byte indices when interacting with the GapBuffer has (unsurprisingly) been one of the main sources of logic bugs within
ad. The GapBuffer itself is highly defensive about this but previous versions of the public GapBuffer API made use of bare usize parameters for indexing which allowed callers to mess things up (particularly when multi-byte utf-8 characters were involved).This PR moves everything over to using a set of index types in order to leverage the type system to give stricter guarantees over how indices are constructed and used. Aiming to fully rule out any invalid indexing is probably going to result in an unwieldy API so instead this is about making it easy to do the right thing and easy to audit for potential logic errors based on how indices are constructed.
The diff (so far) is a lot smaller than I was expecting given that the change is so broad, but most of the non-mechanical changes are confined to the
buffer::internalmodule.Additional changes in this PR
As expected, enforcing this new level of type safety has changed several logic bugs that were possible while everything used
usizeinto type errors. Rather than jump through hoops to keep the current behaviour while making this change, I've opted to include the following bug fixes in this PR.GapBuffer::char_atwas reading from inside the gap when the offset was at the gap start, which could return deleted text./re/and-/re/addresses passed a char offset as a byte offset, so searches started in the wrong place after multi-byte characters.c,<and|edits inEditprograms was incorrect on multi-byte text because the replace delta subtracted a byte span from a char count.narrows-tocompared the message cursor against byte offsets.curis now documented and treated as a character offset.The new index types
There are four distinct ways to index into a GapBuffer depending on the operation in question. The first three offer different ways of performing arbitrary indexing into the contents of the buffer:
RawIdx- A raw byte offset into the memory allocation held by the GapBuffer. This is only permitted within thebuffer::internalmodule as an implementation detail of the GapBuffer and Slice types. Indexing in this way is never guaranteed to return valid utf-8 data so the caller must ensure that indexes are calculated correctly and that the data read is valid.ByteIdx- A byte offset within the logical contents of the GapBuffer. This is used primarily by the regex and treesitter APIs where a GapBuffer is being treated as a byte slice. The standard pitfalls of indexing into a utf-8 encoded string all apply (i.e. care must be taken when multi-byte characters are present) and it is not guaranteed that an arbitrary index of this type lies on a character boundary. However, this type does guarantee that indexing in this way always returns bytes from the logical contents of the buffer and not the gap region.CharIdx- A character offset within the logical contents of the GapBuffer. This is the most expensive index type to work with but it guarantees that indexing in this way always points to a valid utf-8 character boundary. This is used when reading and writing utf-8 data and is efficiently mapped to aRawIdxwhen inserting / deleting at the start of the gap region.The fourth index type is
LineIdxwhich is used to quickly jump to the beginning of newline delimited lines within the logical buffer state.