fix resource_link regex, make non-greedy #927
Conversation
ChristopherChudzicki
requested review from
alicewriteswrongs
and removed request for
alicewriteswrongs
Fixes #917 and #903. Before the regex was greedy so it would not stop at the end of the first resource_link. Two resource links on the same lin would be gobbled up as one. The new regex is still pretty simple but will be fooled by resource_link titles that include literal `" >}}` values. This is a pretty edgy edge case. We could get around this in the future by escpaing quotations in the link titles and using a different regex (or parsing some other way). But right now the unescaped quotation characters are not causing any problems other than this, so lets keep this fix simple.
ChristopherChudzicki
force-pushed
the
regex-fix
branch
from
da0d147 to
64bfc9a
Compare
| ) | ||
| }) | ||
|
|
||
| it("[BUG] does not behave well if link title ends in backslash", async () => { |
There was a problem hiding this comment.
This test was just to document the behavior #928 🤷
| * Limitations: | ||
| * - gets fooled by label texts that include literal `" >}}` values. For | ||
| * example, < resource_link uuid123 "silly " >}} link" >}}. | ||
| */ |
There was a problem hiding this comment.
I think you might have accidentally moved the regex and it's jsdoc comment between the ResourceLinkMarkdownSyntax class declaration and it's jsdoc comment here
There was a problem hiding this comment.
Ahhh silly me. I've moved the regex mostly back up (but put it below the imports)
There was a problem hiding this comment.
one more thing, and sorry I should have been more clear! but it would be good to put the block comment that you wrote for it above the variable declaration too, like this
/**
* Here I can describe
* What something does or doesn't
* do through a haiku
**/
const REGEX = /etc/then various code tools will associate that comment with the REGEX variable as a documentation comment - for instance, in my editor if I ask about the type of a variable it will pop up a little window with type information and also print the associated comment, like this:
There was a problem hiding this comment.
Neat—for some reason I never realized JSDoc comments worked for constants like they do for functions and classes... but why wouldn't they, I suppose.
There was a problem hiding this comment.
yeah, I don't know if they'll work like that for all documentation tools, but they seem to be well supported w/ typescript ecosystem tools at any rate! super handy to have them pop up when something is confusing me later (usually something I wrote originally haha)
Before the regex was greedy so it would not stop at the end of the first resource_link. Two resource links on the same lin would be gobbled up as one.
The new regex is still pretty simple but will be fooled by resource_link titles that include literal
" >}}values. This is a pretty edgy edge case. We could get around this in the future by escpaing quotations in the link titles and using a different regex (or parsing some other way). But right now the unescaped quotation characters are not causing any problems other than this, so lets keep this fix simple.Pre-Flight checklist
What are the relevant tickets?
Fixes #917 and #903.
What's this PR do?
Fixes the regex that was turning resource_link shortcodes into HTM.
How should this be manually tested?
Screenshots (if appropriate)
Before / After
