Skip to content
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

mentionPattern does not match names that end in - #2202

Closed
Askaholic opened this issue Apr 2, 2021 · 8 comments · Fixed by #2258, #2266 or #2271
Closed

mentionPattern does not match names that end in - #2202

Askaholic opened this issue Apr 2, 2021 · 8 comments · Fixed by #2258, #2266 or #2271
Labels
Milestone

Comments

@Askaholic
Copy link
Collaborator

People who's names end in - can't be pinged.
img

After messing around a bit, I think this is due to the \b word boundary matching. As - is not a word character, a space can never match after it (word boundary requires word before and non-word after, or vice versa). You can still ping these people if you put a letter directly after the - in order to trigger the word boundary matching.

Pattern p = Pattern.compile("\\b(" + "Box-" + ")\\b", CASE_INSENSITIVE);
Matcher m = p.matcher("I'm Box-");

System.out.println(m.find());
// Prints "false"

Matcher m2 = p.matcher("I'm Box-x");
System.out.println(m2.find());
// Prints "true"

Wanna have the bug fixed quickly?
Visit Issue hunt...
Issue hunt

@Askaholic Askaholic added the bug label Apr 2, 2021
1-alex98 added a commit that referenced this issue Jun 1, 2021
Sheikah45 pushed a commit that referenced this issue Jun 8, 2021
@Askaholic
Copy link
Collaborator Author

Askaholic commented Jun 8, 2021

The fix is not complete as it doesn’t handle when the username is at the end of the line. It also doesn’t handle the case when the username starts with a dash.

@Askaholic Askaholic reopened this Jun 8, 2021
@1-alex98
Copy link
Member

1-alex98 commented Jun 8, 2021

One should have limited usernames to be alphanumeric only

@Sheikah45
Copy link
Member

Yes I am not sure how much more extendable this is without introducing a bunch of false positive mentions

@Askaholic
Copy link
Collaborator Author

Askaholic commented Jun 8, 2021

I think you can reproduce the behavior of \b but modify the “word” class to include dashes.

Something like ^|[^A-z-]USERNAME[^A-z-]|$ should do the trick.

I also firmly disagree that this is “untestable”. There should be at least a few tests that check stuff like:

  • Box- matches Box-
  • -Box matches -Box
  • cardboard-Box does not match -Box
  • Box-boy does not match Box-
  • Hello Box-! matches Box-

@1-alex98
Copy link
Member

1-alex98 commented Jun 9, 2021

I think you can reproduce the behavior of \b but modify the “word” class to include dashes.

Something like ^|[^A-z-]USERNAME[^A-z-]|$ should do the trick.

I also firmly disagree that this is “untestable”. There should be at least a few tests that check stuff like:

  • Box- matches Box-
  • -Box matches -Box
  • cardboard-Box does not match -Box
  • Box-boy does not match Box-
  • Hello Box-! matches Box-

The actual code this is in

@Askaholic
Copy link
Collaborator Author

Btw using A-z for the character class was lazy phone typing and will include a few special characters like square brackets, which we probably don’t want.

@Sheikah45 Sheikah45 reopened this Jun 12, 2021
@Sheikah45
Copy link
Member

Sheikah45 commented Jun 12, 2021

@Askaholic this pattern matches on just spaces as did @1-alex98. So this is not trivial it seems

@Sheikah45
Copy link
Member

I think I was able to fix it

@Sheikah45 Sheikah45 added this to the v1.4.7 milestone Jun 20, 2021
mrchris2000 pushed a commit to mrchris2000/downlords-faf-client that referenced this issue Apr 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants