Skip to content

Refactor Zig code to use array type instead of bare arrays all over the place#220

Merged
emil-e merged 1 commit into
dakra:mainfrom
emil-e:emil-e/array
May 3, 2026
Merged

Refactor Zig code to use array type instead of bare arrays all over the place#220
emil-e merged 1 commit into
dakra:mainfrom
emil-e:emil-e/array

Conversation

@emil-e
Copy link
Copy Markdown
Collaborator

@emil-e emil-e commented May 2, 2026

Safer code is better code. Chose not to use Zig builtin types because the ergonomics are poor and require the buffer to be separate.

@emil-e
Copy link
Copy Markdown
Collaborator Author

emil-e commented May 3, 2026

@dakra Any input or should I just merge?

@dakra
Copy link
Copy Markdown
Owner

dakra commented May 3, 2026

For me the code looks great. Some claude review comments, do with them what you like:

  • ghostel--has-wide-chars is now set per-row instead of once per render. Old code aggregated has_wide_chars across rows and called env.call2(intern("set"), …, t) exactly once after the loop (gated on dirty != DIRTY_FALSE). New code calls it inside insertAndStyle for every dirty row that contains a wide cell. Functionally equivalent (idempotent set), but it adds an intern + call2 per such row. Minor, but trivially recoverable: keep the old return-the-flag pattern, or stash the

fixed_array_list.zig

  • Consider std.BoundedArray(T, N) from the stdlib — it's the same idea (inline buffer + len, append/slice/constSlice/resize, error.Overflow). The PR description says builtin types have "poor ergonomics," but BoundedArray is exactly the shape you built. If there's a concrete reason it doesn't fit (missing method, Zig 0.15.2 API gap), worth a one-line comment in the file; otherwise dropping the helper would save ~180 LOC.
  • unusedCapacitySlice simplifies to self.buffer[self.len..capacity]. The self.len + self.unusedCapacity() form is correct
    but indirect.
  • Typo: // Returns a slice of the reamining capacity. → remaining (src/fixed_array_list.zig:39).
  • items() and constItems() have identical doc comments; one says "mutable" the other "read-only" would help.
  • Missing blank line between the last two tests (constItems and resize: shrinks…), src/fixed_array_list.zig:171.

@emil-e emil-e merged commit 4281d26 into dakra:main May 3, 2026
20 checks passed
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