Skip to content

use longest match instead of endsWith#5146

Draft
simonLeary42 wants to merge 4 commits into
OSC:masterfrom
simonLeary42:refactor-dynamic-forms7
Draft

use longest match instead of endsWith#5146
simonLeary42 wants to merge 4 commits into
OSC:masterfrom
simonLeary42:refactor-dynamic-forms7

Conversation

@simonLeary42
Copy link
Copy Markdown
Contributor

@simonLeary42 simonLeary42 commented Mar 10, 2026

idFromToken expects the token argument to be a form element ID in MountainCase, but the argument can actually include arbitrary trailing characters:

let match = str.match(`^${token}{1}`);

But then later during an edge case, idFromToken expects an exact match:

return element.endsWith(snake_case_str);

In this case, trailing characters would result in an index-out-of-bounds error. A more reliable way to handle this edge case would be to compare the string length of the matches.

I am assuming that there is a legitimate use case for arbitrary trailing characters. If not, this is completely pedantic. A $ could be added to the end of the regex, but it doesn't make much of a difference. The test case still might be worth keeping, though.

@simonLeary42
Copy link
Copy Markdown
Contributor Author

simonLeary42 commented Mar 10, 2026

marked as draft until I add test cases

@simonLeary42 simonLeary42 marked this pull request as ready for review May 27, 2026 15:53
@simonLeary42 simonLeary42 marked this pull request as draft May 27, 2026 16:06
@simonLeary42
Copy link
Copy Markdown
Contributor Author

simonLeary42 commented May 27, 2026

Actually, I don't think this is correct. If your token is cluster, and you match both cluster and cluster_file_system, then the string length of the match will be the same for both.

edit: Actually it's fine. You can't match cluster_file_system if you search for cluster. The form element ID is allowed to be a substring of the token argument, not the other way around. cluster_file_system is not a substring of cluster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants