log icon indicating copy to clipboard operation
log copied to clipboard

Fixed `Message::parse()` regex to handle both multiple placeholders and deeply nested ones

Open technicated opened this issue 1 month ago • 7 comments

Fix multiple placeholders replacement in Message::parse().

This PR fixes an issue where multiple placeholders were not replaced inside Message::parse().

The approach suggested in the linked issue (making the regex lazy rather than greedy) proved insufficient, as it broke valid cases involving nested placeholders and causes existing tests to fail.

To correctly handle balanced {} pairs at arbitrary nesting depth, the placeholder-matching regex has been updated to use PCRE recursion. This ensures that only the outermost placeholder is matched while allowing nested braces to be treated as content.

Recursive regexes can have performance implications due to backtracking. In this case, I considered the trade-off to be acceptable because:

  • Message inputs are expected to be short/medium in length
  • they should be programmer-generated, not user-controlled
  • deep nesting, while supported, is not a realistic use case

Given these constraints, the recursive solution provides the most correct and maintainable fix.

If instead support for nested braces is not considered a requirement for this library, the simpler lazy (non-recursive) regex should be sufficient. The recursive solution was chosen here to preserve existing behavior and test coverage while fixing the reported issue.

Q A
Is bugfix? ✔️
New feature?
Breaks BC?
Fixed issues #130

technicated avatar Jan 15 '26 11:01 technicated