Combine the suffixes for shorter regular expression. #1651
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
akirk
approved these changes
Jul 11, 2023
Member
akirk
left a comment
akirk
left a comment
There was a problem hiding this comment.
Works well, I've added unit tests.
pedro-mendonca
added a commit
to pedro-mendonca/GlotPress
that referenced
this pull request
Aug 26, 2023
* Combine the suffixes for shorter regular expression. * Whitespace * Add unit tests --------- Co-authored-by: Alex Kirk <[email protected]> # Conflicts: # tests/phpunit/testcases/test_template_helper_functions.php
amieiro
added a commit
that referenced
this pull request
Jan 10, 2024
* Add 'ed', 'ion', and 'ing' to suffix list for glossary term ending with 'e' * Add known suffixes for nouns and verbs * Revert previous commits to further rebase * Add known suffixes for nouns and verbs * Replace the whole list of known plurals by part_of_speech. * Only trims ending pipe * Check if key term is set and is an array * Make suffix doc more clear * Add unit tests for suffixes * Fix tests descriptions * Rename variables for a generic approach for nouns and verbs * Improve list of Nouns formed by verbs with the suffix -ion * Move 'changes' to the pairs ending => change * Improve suffixes changes documentation * Test verbs that form nouns ending with -tion * Test verbs that form nouns ending with -sion * Remove unnecessary code * Fix endings documentation and some formatting * Apply coding standards * Add link to the plurals formation documentation * Improve suffix comment * Don't duplicate endings to glossary entry suffixes. * Combine the suffixes for shorter regular expression. (#1651) * Combine the suffixes for shorter regular expression. * Whitespace * Add unit tests --------- Co-authored-by: Alex Kirk <[email protected]> # Conflicts: # tests/phpunit/testcases/test_template_helper_functions.php * We don't need it anymore because the match checks what precedes the ending * Fix part of speech for its own set of endings * Make the Suffix list filterable. * Rename filter and improve description * Fix typo in tests part_of_speech introduced in PR #1706 * Remove matching for irregular Nouns ending with '-an' * Improve legibility and consistency of dockblocks comments * Add support for verbs ending with single sibilant '-s' (bias) * Fix error in ending size * Improve glossary matches doc comments * Change matches array keys order, preceded first * Use actual preceded for any letters instead of null value. * Add support for placeholders in the endings, to allow reuse of the current ending * Add matches that double the ending consonant * Add check_map_glossary() to test different glossary matches * Fix comment. * Add all the glossary matches tests using check_map_glossary() * Remove irregular Noun plural match * Add test for verb focus * Fixed comments * Add data files for manual testing * Remove ending newlines in example data file * Add glossary matching case example in docblock * Remove glossary suffix .pot and .csv example files * Add check_map_glossary() docblock types --------- Co-authored-by: Jesús Amieiro Becerra <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem
Per #1647 large glossaries cause the expression to fail to execute when the glossary item count is high.
This is due to regular expressions having a limit to the overall length.
Solution
This solution is not the most ideal, ideally this should not be using a regular expression at all.
This solution simply combines the regex such that the suffix matches are only included once, rather than individually on every single term.
For Translate.WordPress.org glossaries, it reduces the regex length...
Roughly a 50-60% decrease in length.
Initially there were no logical changes I could see, but now that I'm PR'ing it, I can see that there's a "small" change - the terms are no longer sorted by length, as they're only sorted within their "suffix group".
\b(favorited(?:s|es|ed|ing)?|favorites(?:s|es|ed|ing)?|zip code|url(?:s|es|ed|ing)?)\b\b((?:favorited|favorites|url)(?:s|es|ed|ing)?)|(?:zip code))\b(Yes, those suffixes are wildly inaccurate, but that's not the purpose of this issue/pr)
To-do
Testing Instructions
Screenshots or screencast