Skip to content

Unify placeholder modifiers - enable formatters as modifiers#10

Closed
Copilot wants to merge 7 commits intomainfrom
copilot/unify-placeholder-modifiers
Closed

Unify placeholder modifiers - enable formatters as modifiers#10
Copilot wants to merge 7 commits intomainfrom
copilot/unify-placeholder-modifiers

Conversation

Copy link
Contributor

Copilot AI commented Feb 5, 2026

Plan: Unify Placeholder Modifiers ✅

  • Analyze existing code structure
    • Understand Modifier interface and chain of responsibility pattern
    • Understand FormatterBuilder's dynamic formatter instantiation
    • Understand pipe argument parsing in existing modifiers (e.g., list:or)
  • Design FormatterModifier implementation
    • Create FormatterModifier class that parses pipe syntax
    • Parse formatter name and arguments from pipe (e.g., date:Y-m-d → DateFormatter with 'Y-m-d' arg)
    • Use reflection logic to instantiate formatters dynamically
    • Integrate with existing modifier chain
    • Fix escaping for colons in arguments (use preg_split with negative lookbehind)
    • Fix regex to handle escaped pipes in PlaceholderFormatter
  • Write comprehensive tests
    • Test basic formatter usage as modifiers (date, number, mask, etc.)
    • Test formatter arguments via colon syntax
    • Test fallback to next modifier when formatter not found
    • Test string passthrough for non-string values
    • Integration tests with real-world templates
    • Tests for escaped colons and pipes in arguments
  • Update PlaceholderFormatter default modifier chain
    • Add FormatterModifier to default chain
    • Ensure backward compatibility with existing modifiers
  • Code quality
    • Check syntax of all code files
    • Fix code style issues (PSR-12 compliance)
    • Code review completed with no issues
    • CodeQL security check (not applicable for PHP)
    • Address reviewer feedback on escaping
  • Documentation
    • Add comprehensive FormatterModifier documentation
    • Update PlaceholderFormatter docs with examples
    • Update Modifiers.md to list FormatterModifier
    • Update README with usage examples
    • Document escaping special characters (: and |)

Summary of Changes

This PR implements unified placeholder modifiers as requested in the issue, allowing all formatters to be used as modifiers without creating dedicated modifier classes.

Key Changes:

  1. Created FormatterModifier that dynamically instantiates formatters based on pipe syntax
  2. Updated PlaceholderFormatter to include FormatterModifier in default chain
  3. Proper escaping support for special characters in arguments (\: and \|)
  4. Comprehensive tests including escaping scenarios
  5. Complete documentation with examples
Original prompt

This section details on the original issue you should resolve

<issue_title>Unify placeholder modifiers</issue_title>
<issue_description>We have lots of formatter right now (Hooray!), and ideally we'd have as many placeholder formatters. It's not ideal to have once modifiers class for each formatters.

I would like to have a general modifier that would parse any placeholder modifier and the FormatterBuilder::__callStatic() (or something else) execute a formatter as a placeholder modifier. The Modifier interface remains useful, because we do not want to tight the modifiers to the formatters and not all of them deal with strings, but want to have one modifier implementation that's coupled to our own formatters.

Modifiers already support arguments via subsequent : after the pipe, like list:or, we just need to make those arguments values to pass to the construtor of the formatters.</issue_description>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 4 commits February 5, 2026 13:17
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Copilot AI changed the title [WIP] Unify placeholder modifiers for formatters Unify placeholder modifiers - enable formatters as modifiers Feb 5, 2026
Copilot AI requested a review from henriquemoody February 5, 2026 13:34
Copy link
Member

@henriquemoody henriquemoody left a comment

Choose a reason for hiding this comment

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

This is a great start!

We need to change the PlaceholderFormatter because right now, it's splitting the modifier to the placeholder any time it finds a |, but now that you're allowing different arguments to modifiers, they could also contain a |, for example:

{{placeholder|pattern:###\|###}}

In that case, pattern:###|### should be the pipe, but if you split it into every |, the value of $pipe could be ###. We need to at least create a test to verify if that works as expected.

private function tryCreateFormatter(string $name, array $arguments): Formatter|null
{
/** @var class-string<Formatter> $class */
$class = 'Respect\\StringFormatter\\' . ucfirst($name) . 'Formatter';
Copy link
Member

Choose a reason for hiding this comment

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

YOu can just use the FormatterBuilder::__callStatic() here

Copilot AI and others added 2 commits February 5, 2026 13:41
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Co-authored-by: henriquemoody <154023+henriquemoody@users.noreply.github.com>
Copy link
Contributor Author

Copilot AI commented Feb 5, 2026

This is a great start!

We need to change the PlaceholderFormatter because right now, it's splitting the modifier to the placeholder any time it finds a |, but now that you're allowing different a...

Fixed in commits ac22467 and 8f9f056. Updated the PlaceholderFormatter regex to handle escaped pipes: /{{(\w+)(\|([^}\\\\]*(?:\\\\.[^}\\\\]*)*))?}}/. Also added comprehensive tests for both escaped colons and pipes, plus updated documentation with examples.

@henriquemoody
Copy link
Member

Closed in favor of #12

@henriquemoody henriquemoody deleted the copilot/unify-placeholder-modifiers branch February 6, 2026 20:43
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.

Unify placeholder modifiers

2 participants