Skip to content

Conversation

@SilasArche
Copy link

Closes #963

Addresses issue #963.

Previously, the InlineParserEngine checked $node->data->has('delim') every time it attempted to merge adjacent text nodes. This resulted in frequent array lookups in the hot path of the parser.

This commit promotes the delimiter status to a first-class property on the Text node. The constructor now strips the delim key from the $data array and sets the boolean property $isDelimiter instead.

This reduces overhead for text merging operations without changing the public API surface for extension authors who instantiate Text nodes.

"We polish the silver." - The Wake Guild

Copy link
Member

@colinodell colinodell left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

I think I'd prefer to fully switch to using the new property, though - no getters, setters, or lingering support for the delim property anywhere in the codebase. Would you be open to making those changes?

@SilasArche SilasArche force-pushed the perf/text-delimiter-property branch from 28e8c37 to 2caec47 Compare January 24, 2026 19:18
@SilasArche SilasArche requested a review from colinodell January 24, 2026 19:20
@SilasArche SilasArche force-pushed the perf/text-delimiter-property branch from 2caec47 to 0bbec4a Compare January 26, 2026 23:27
Accessing the `delim` flag within the `$data` array on `Text`
nodes is a performance bottleneck in the inline parser engine.
This change promotes the flag to a first-class public property
for faster access.

BREAKING CHANGE:
The internal `delim` key in the `$data` array has been removed.
The `isDelimiter()` and `setDelimiter()` methods have also been
removed. Consumers must now directly access the new `public bool
$isDelimiter` property on `Text` nodes.

Fixes thephpleague#963

"We polish the silver."
@SilasArche SilasArche force-pushed the perf/text-delimiter-property branch from 0bbec4a to efaa0b7 Compare January 26, 2026 23:52
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.

Promote 'delim' to property

2 participants