Cache regex matches and add ends_on option for Syntax definitions

[defname]

Summary by CodeRabbit

• New Features

  • Added buffer clear functionality and space availability check.
  • Introduced "ends on" block relationships for enhanced syntax definition handling.

• Bug Fixes & Improvements

  • Implemented regex match caching for improved highlighting performance.
  • Enhanced syntax block termination logic for better accuracy.

• Refactor

  • Strengthened const-correctness across buffer read-only operations.

• Tests

  • Added comprehensive test coverage for new block relationship features and highlighting scenarios.

[defname]

@coderabbitai review

[coderabbitai]

Walkthrough

This PR enhances the Buffer API with const-correctness improvements and extends the syntax definition/highlighting system by introducing an "ends_on" block relationship mechanism. It adds a caching layer for regex matching in the highlighting module with offset-aware matching, updates syntax definition linking to resolve ends_on relationships after child relationships, and adds corresponding test coverage.

Changes

Cohort / File(s)	Summary
Buffer API const-correctness src/common/buffer.h, src/common/buffer.c	Added Buffer_Clear() function; made query/accessor functions (Buffer_IsEmpty(), Buffer_Size(), Buffer_Capacity(), Buffer_Peek()) accept const Buffer*; added Buffer_HasSpace() accessor.
Syntax definition ends_on relationships src/syntax/definition.h, src/syntax/definition.c	Introduced ends_on and ends_on_count members to SyntaxBlockDef; added MatchCache struct; implemented block name resolution and linking for ends_on relationships after child linkage; updated memory management for new structures.
Syntax highlighting with regex caching src/syntax/highlighting.c	Added caching layer via regexec_with_cache() and per-block caches; introduced helpers (find_first_block(), find_first_ends_on_block()) to detect first-match across block relations; modified highlight loop to compare cached child/ends_on/end matches with offset-aware scanning.
Test updates tests/test_syntax_definition.c, tests/test_syntax_highlighting.c	Replaced allowed_blocks with child_blocks in test data; added test_ends_on() for ends_on relationship validation; added test_moderate() for complex block interactions; extended test_basics() with loop-based case iteration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

The changes span multiple subsystems with mixed complexity: Buffer API updates are straightforward const-correctness improvements, but the ends_on mechanism introduces new definition-parsing phases and memory management, while the highlighting caching layer adds offset-aware regex matching and new block-relation handling. The heterogeneous nature and density of highlighting logic warrant moderate review attention.

Possibly related PRs

• clieditor#85: Modifies the syntax highlighting subsystem with regex and block-matching logic directly related to this caching/ends_on enhancement.
• clieditor#83: Updates syntax subsystem including SyntaxBlockDef and parsing/highlighting logic, overlapping with the ends_on relationship changes here.

Poem

🐰 A buffering bunny hops so bright,
Const-correctness shines just right!
Blocks end_on blocks in harmony,
Regex caches hop with glee,
Syntax syntax, now it's free! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Cache regex matches and add ends_on option for Syntax definitions" accurately captures the two primary feature additions in the changeset. The summary of changes clearly shows the introduction of regex match caching via the new MatchCache struct and regexec_with_cache function in the highlighting module, as well as the new ends_on relationship system for syntax blocks with corresponding members (ends_on, ends_on_count) added to SyntaxBlockDef. The title is concise, specific, and avoids vague terminology. While the changeset also includes supporting infrastructure improvements to the Buffer API and test updates, these are secondary to the main feature additions, and the title appropriately prioritizes the most significant changes.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch refactor/sh-precalculate-block-matches

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/syntax/definition.h (2)

68-72: Missing headers for bool and ssize_t cause portability/compile failures.
Include stdbool.h and sys/types.h in this public header.

Apply this diff:

 #include <stdint.h>
 #include <regex.h>
+#include <stdbool.h>
+#include <sys/types.h>
 #include "common/table.h"
 #include "common/string.h"

---

41-63: Header docs out of date (allowed_blocks vs child_blocks; add ends_on).
Update narrative to use child_blocks and describe ends_on. Also note root semantics accordingly.

Apply this diff (adjust wording as needed):

- * Blocks that define an `end` expression may additionally define a list of
- * `allowed_blocks`, which specifies which block types are permitted to appear
- * inside the block.
+ * Blocks that define an `end` expression may additionally define
+ * `child_blocks`, which specifies which block types may appear inside.
+ * A block can also define `ends_on`, a list of block types whose start
+ * implicitly terminates the current block.
  * 
- * The root block does not need defined `start` or `end` patterns but should
- * define an `allowed_blocks` list (otherwise nothing will be found). 
+ * The root block does not need `start` or `end` patterns but should define
+ * `child_blocks` (otherwise nothing will be found).

🧹 Nitpick comments (13)

tests/test_syntax_highlighting.c (2)

156-257: Table‑driven tests: good coverage; add one equality-edge case.
Consider adding a case where a child start and a parent end coincide (same byte_offset) to pin down tie-breaking. Current logic uses strict “<” and will prefer the end over the child when equal.

---

258-279: Ends-on test INI looks correct; add no-comment variant.
Add a case like "foo=value" (no comment) to ensure value runs to EOL and assignment closes as expected.

tests/test_syntax_definition.c (1)

211-238: Ends-on linkage test is sound; consider verifying actual targets.
Optionally assert that block1->ends_on[0/1]->name are "block3"/"block2" (order preserved) to catch mapping mistakes.

src/syntax/definition.h (2)

102-110: MatchCache API exposure: document lifetime and thread-safety.
Since caches live in SyntaxBlockDef, clarify they’re per-highlight run and not thread-safe; consumers must not share a SyntaxDefinition across concurrent highlight calls without external synchronization.

---

125-132: Public struct growth: ensure zero-init in creators.
start_cache/end_cache aren’t initialized in SyntaxBlockDef_Create(); highlighting.c does init them before use, but add explicit zero-init here to prevent accidental misuse elsewhere.

Apply this diff:

 SyntaxBlockDef *SyntaxBlockDef_Create();

And in the corresponding implementation (definition.c):

 block->children_count = 0;
 block->ends_on = NULL;
 block->ends_on_count = 0;
+memset(&block->start_cache, 0, sizeof(block->start_cache));
+memset(&block->end_cache, 0, sizeof(block->end_cache));

src/syntax/highlighting.c (5)

102-120: regexec_with_cache: semantics OK; guard comment for ‘done’.
Add a brief comment that once a pattern is “done”, no further matches can occur for increasing offsets because regexec scans the remainder. Avoids future regressions.

---

156-169: Cache init is correct; small nit: use memset for brevity.
Optional style tweak.

Apply this diff:

-        block->start_cache.match.rm_so = -1;
-        block->start_cache.match.rm_eo = -1;
-        block->start_cache.offset = 0;
-        block->start_cache.done = false;
-        block->end_cache.match.rm_so = -1;
-        block->end_cache.match.rm_eo = -1;
-        block->end_cache.offset = 0;
-        block->end_cache.done = false;
+        memset(&block->start_cache, 0, sizeof(block->start_cache));
+        block->start_cache.match.rm_so = -1;
+        block->start_cache.match.rm_eo = -1;
+        memset(&block->end_cache, 0, sizeof(block->end_cache));
+        block->end_cache.match.rm_so = -1;
+        block->end_cache.match.rm_eo = -1;

---

218-221: Define tie-breaking explicitly (equal offsets).
Current checks use strict “<”. On equal offsets, end wins over child, and end wins over ends_on. If this is intended, add a comment; if not, switch to “<=” accordingly.

Also applies to: 237-239, 258-272

---

171-183: Thread-safety: caches live in SyntaxBlockDef.
If the same SyntaxDefinition is highlighted concurrently, caches will race. Either:

• document non-thread-safety and require external synchronization, or
• move caches into a per-call structure keyed by SyntaxHighlightingString.

Also applies to: 194-197

---

51-65: Minor: preallocate tags capacity once.
You allocate tags on Create and then realloc on first add because tags_capacity is 0. Initialize tags_capacity to SHS_TAGS_INITIAL_CAPACITY in Create to avoid the immediate realloc.

Apply this diff:

 shs->tags = NULL;
 shs->tags_count = 0;
-shs->tags_capacity = 0;
+shs->tags_capacity = SHS_TAGS_INITIAL_CAPACITY;
 ...
-shs->tags = malloc(sizeof(SyntaxHighlightingTag) * SHS_TAGS_INITIAL_CAPACITY);
+shs->tags = malloc(sizeof(SyntaxHighlightingTag) * shs->tags_capacity);

src/syntax/definition.c (3)

220-241: Name mapping: message uses old term.
Error comment still references allowed_blocks; update to child_blocks (and/or ends_on) to match current keys.

Apply this diff:

-            // something like
-            // allowed_blocks = block1, non_existing_block
+            // e.g., child_blocks = block1, non_existing_block

---

243-281: block_name_list_str_to_blocks: free on error is safe but be explicit.
On error, you already return and SyntaxDefinition_Destroy will free block->children/ends_on. Add a brief comment to clarify the ownership to future readers.

---

358-365: Comment nit: “construct the ends_by list” → “ends_on list”.

Apply this diff:

-    // construct the ends_by list
+    // construct the ends_on list

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 59c477a and d5f3f1b.

📒 Files selected for processing (7)

• src/common/buffer.c (2 hunks)
• src/common/buffer.h (3 hunks)
• src/syntax/definition.c (5 hunks)
• src/syntax/definition.h (3 hunks)
• src/syntax/highlighting.c (4 hunks)
• tests/test_syntax_definition.c (4 hunks)
• tests/test_syntax_highlighting.c (5 hunks)

🔇 Additional comments (16)

src/common/buffer.h (3)

51-54: LGTM! Clear API addition.

The Buffer_Clear function provides a straightforward way to reset the buffer state without deallocation.

---

78-78: Excellent const-correctness improvements.

Making these read-only accessors accept const Buffer * is the correct approach and prevents accidental mutations.

Also applies to: 81-81, 84-84, 96-96

---

86-87: LGTM! Useful utility function.

The Buffer_HasSpace declaration is appropriate. Will verify the implementation logic in buffer.c.

src/common/buffer.c (3)

42-45: LGTM! Clean buffer reset.

The implementation correctly resets the buffer state by clearing the count and cursor. Not zeroing the actual memory is appropriate for performance.

---

73-73: LGTM! Const-correctness properly applied.

The implementations correctly match the updated header declarations with const qualifiers.

Also applies to: 77-77, 81-81, 89-89

---

94-96: Good defensive check added.

Adding the NULL guard for out_element prevents undefined behavior. This changes the function to return false instead of crashing when NULL is passed, which is a reasonable defensive improvement.

tests/test_syntax_highlighting.c (4)

99-103: LGTM: switch to child_blocks in test_ini0 is consistent with parser changes.

---

137-143: LGTM: updated child_blocks lists reflect nesting rules used in tests.

---

281-291: LGTM: moderate test validates ends_on transition and tag ordering at the same final offset.

---

394-394: Suite registration: OK.

tests/test_syntax_definition.c (3)

161-161: LGTM: error cases now use child_blocks and assert SYNTAXDEFINITION_BLOCK_DOES_NOT_EXIST.

Also applies to: 169-169

---

193-193: LGTM: children linkage test updated for child_blocks.

---

247-247: Suite registration: OK.

src/syntax/definition.c (3)

50-52: ends_on memory management: OK.

Also applies to: 60-63

---

294-301: child_blocks key linkage: OK.

---

303-321: ends_on linkage: OK.