-
Notifications
You must be signed in to change notification settings - Fork 129
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
Filter the placeholders to show them with the glossary words #1696
Conversation
Co-authored-by: Alex Kirk <[email protected]>
To test the overhead in load time that this new regex might add, I used a Bash script to test it: Before, I have:
This is the script (change your local URL to test it):
This is the output after 100 executions:
I have obtained similar outputs with another executions, so no overload is apparent. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for doing the performance tests, looks good if it performs at the same speed!
Problem
Imagine we have the
show
word in the glossary. If we have a placeholder in an original string finishing with thes
letter and, without space, we have thehow
word:%show
, the current algorithm detects the glossary word and not the placeholder, showing on mouse over a popover with the glossary information, instead of detects the%s
placeholder and thehow
word.%show
->%
+show
%show
->%s
+how
Fixes #1400.
Solution
This solution updates the regex that split the original string into an array, to isolate the placeholders, avoiding mixing parts of them with other substrings.
The regex used is the same used to detect warnings with the placeholders.
Testing Instructions
This PR adds two tests in the
tests/phpunit/testcases/test_template_helper_functions.php
file:test_map_glossary_entries_with_placeholders_glued_glossary_words
test_map_glossary_entries_with_placeholders_glued_glossary_words_in_the_plural_origin