Plots2: @mentions can break URLs that include a @ character

Created on 6 Sep 2018  Â·  5Comments  Â·  Source: publiclab/plots2

We have a great system which turns instances of @______ into a link, just like on GitHub (like @jwarren). However, it breaks for string like:

[From the Bayou to the Bay: Voices of Gulf Resistance](https://medium.com/@350/from-the-bayou-to-the-bay-voices-from-the-gulf-f6707c7ecff3)"

I believe even without the Markdown link syntax, that URL would break:

https://medium.com/@350/from-the-bayou-to-the-bay-voices-from-the-gulf-f6707c7ecff3

I think we need to modify the regular expression here:

https://github.com/publiclab/plots2/blob/3f0bdf2bb12a89d72a05a3d52738f2d8d41d3aeb/config/initializers/constants.rb#L2

so that it doesn't detect a name if it has a non-whitespace character immediately before it. But let's be careful -- it should still work with @ as the first character of a string, or first of a new line.

https://Rubular.com is a great place to test this out. Here i've made a test string that shows a bunch that SHOULD match, with one that shouldn't (in the URL):

http://rubular.com/r/Lxx21V5SJN

Experimenting with this and reading the RegEx documentation should be a good way to solve this, and we can even turn my example into a test here:

https://github.com/publiclab/plots2/blob/master/test/unit/constants_test.rb

Ruby bug help wanted testing

Most helpful comment

Hello, I would like to work on this issue.

All 5 comments

Hello, I would like to work on this issue.

Hi @cthulhu-uu , that would be awesome. Please go ahead and if you have any questions feel free to ask here. Thanks!

Thanks so much! I did have a question to help clarify the requirements:

The issue description says that we need to make it so that the mention doesn't detect a name if it doesn't have a whitespace preceding it, but the test case that the author wrote says "@warren/@warren" should work, which seems to contradict the issue description.

So that I may narrow down my testing (sorry, still new to programming!), are both of these specifications required? Should the solution make it so that both the mentions in the test case (@warren/@warren) work AND not detect in a URL? Or should the solution make it so that the mention doesn't work if not preceded by a non-whitespace character (in which case, only the first mention in "@warren/@warren" would match)?

I think the @jywarren/@jywarren case is extra difficult - ideally it does
match both - but also it's a rare case. If you don't find a good solution
to cover this case we can open a follow up issue with it because it's
pretty rare, and focus on the much more common case that the issue mainly
describes. Thank you!

On Sat, Sep 8, 2018, 6:47 AM cthulhu-uu notifications@github.com wrote:

Thanks so much! I did have a question to help clarify the requirements:

The issue description says that we need to make it so that the mention
doesn't detect a name if it doesn't have a whitespace preceding it, but the
test case that the author wrote says "@warren/@warren
https://github.com/warren" should work, which seems to contradict the
issue description.

So that I may narrow down my testing (sorry, still new to programming!),
are both of these specifications required? Should the solution make it so
that both the mentions in the test case (@warren/@warren
https://github.com/warren) work AND not detect in a URL? Or should the
solution make it so that the mention doesn't work if not preceded by a
non-whitespace character (in which case, only the first mention in "@warren/
@warren https://github.com/warren" would match)?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/publiclab/plots2/issues/3303#issuecomment-419631874,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABfJ95aI5Civsa3dxRHVYiUFOHliwF9ks5uY6AlgaJpZM4WddQC
.

Hi, @cthulhu-uu -- thanks for all your help on this!

If you're interested afterwards, I posted a kinda similar sort of issue over here: https://github.com/publiclab/plots2/issues/3308

Just in case you're looking for something new after that!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

milaaraujo picture milaaraujo  Â·  3Comments

jywarren picture jywarren  Â·  3Comments

divyabaid16 picture divyabaid16  Â·  3Comments

noi5e picture noi5e  Â·  3Comments

keshavsethi picture keshavsethi  Â·  3Comments