Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow more than one character to be used as a symbol for MentionExtension #550

Merged
merged 1 commit into from
Oct 3, 2020

Conversation

colinodell
Copy link
Member

Thanks to #549 we're able to implement this for the upcoming 2.0 release

Resolves #514

@colinodell colinodell added the enhancement New functionality or behavior label Sep 26, 2020
@colinodell colinodell added this to the v2.0 milestone Sep 26, 2020
@colinodell colinodell self-assigned this Sep 26, 2020
@colinodell colinodell force-pushed the multi-character-mention-symbols branch 2 times, most recently from 37d3f1f to 7f24cce Compare September 26, 2020 22:34
@@ -70,7 +70,7 @@ public function parse(string $match, InlineParserContext $inlineContext): bool
$previousState = $cursor->saveState();

// Advance past the symbol to keep parsing simpler
$cursor->advance();
$cursor->advanceBy(\mb_strlen($this->symbol));

// Parse the identifier
$identifier = $cursor->match($this->mentionRegex);
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like both the symbol and mention regex should be combined into a single regex that uses parenthesis to group the individual results. Maybe something like:

        return InlineParserMatch::join(
          InlineParserMatch::string($this->symbol),
          InlineParserMatch::regex($this->mentionRegex),
        );

Unfortunately, this would also mean that string $match would have to really be array $matches.

Also, is there a reason we can't add both of these to InlineParserContext, i.e. $inlineContext->getMatch(): string and $inlineContext->getMatches(): array; the former just returnining the first item of the latter?

Then we could something like the following here:

        // Extract the symbol and identifier from the matches.
        [$symbol, $identifier] = $inlineContext->getMatches();

        // Advance past the symbol to keep parsing simpler
        $cursor->advanceBy(\mb_strlen($symbol));

Copy link
Member Author

Choose a reason for hiding this comment

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

Great idea! I've incorporated this into #560.

@colinodell colinodell force-pushed the multi-character-mention-symbols branch from 7f24cce to 2f57dc0 Compare October 3, 2020 14:45
@colinodell colinodell mentioned this pull request Oct 3, 2020
@colinodell colinodell merged commit 2f57dc0 into latest Oct 3, 2020
@colinodell colinodell deleted the multi-character-mention-symbols branch October 3, 2020 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality or behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow more than one character to be used as a symbol for MentionExtension.
2 participants