After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 756761 - @Mentions don't notify the user
@Mentions don't notify the user
Status: RESOLVED FIXED
Product: polari
Classification: Applications
Component: general
3.18.x
Other Linux
: Normal normal
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2015-10-17 19:24 UTC by tirifto
Modified: 2015-10-18 14:08 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Highlight message on special characters preceding nick (1.17 KB, patch)
2015-10-18 09:21 UTC, Kunaal Jain
committed Details | Review

Description tirifto 2015-10-17 19:24:41 UTC
Polari normally sends me a notification when someone mentions my name. This usually works with additional character, for example "User,". However, mentions don't work with the character "@", so if someone writes "@User", I don't get notified.

The "@" character worked fine in earlier versions.
Comment 1 Florian Müllner 2015-10-17 20:02:31 UTC
(In reply to tirifto from comment #0)
> The "@" character worked fine in earlier versions.

Yes, previous versions simply highlighted any occurrences of the nick anywhere in a message, which resulted in painfully many false positives for some nicks (for instance in gnome, we have "aday" and "av" who really hated any mentions of "nowadays", "available", "raving" etc. :-) )
The new rule is that the match has to be either at the start of the message or preceded by a whitespace - extending that to allow '@' (and possibly other characters?) as well makes sense to me.
Comment 2 Kunaal Jain 2015-10-18 07:51:04 UTC
(In reply to Florian Müllner from comment #1)
> The new rule is that the match has to be either at the start of the message
> or preceded by a whitespace - extending that to allow '@' (and possibly
> other characters?) as well makes sense to me.

maybe like we are doing in case of word ending, same way in word starting with g_ascii_isspace (*(match - 1)) use !g_ascii_isalnum (*(matchi-1)). This way we are blocking all mentions in middle of words but highlight all mentions when special characters either precede (space too!) or succeed..
Comment 3 Kunaal Jain 2015-10-18 08:38:53 UTC
Update: I have been testing, !g_ascii_isalnum looks good to me. Not uploading patch since my previous commit is still pending review. You can test on wip/kunaljain/bug-fixes branch.
Comment 4 Florian Müllner 2015-10-18 08:55:31 UTC
(In reply to Kunaal Jain from comment #3)
> Not uploading patch since my previous commit is still pending review.

Hmm? (1) there is no previous patch here (2) if you do update an existing patch, you should replace it to not let people review outdated code
Comment 5 Kunaal Jain 2015-10-18 09:21:44 UTC
Created attachment 313606 [details] [review]
Highlight message on special characters preceding nick
Comment 6 Florian Müllner 2015-10-18 09:22:37 UTC
Review of attachment 313606 [details] [review]:

LGTM
Comment 7 Kunaal Jain 2015-10-18 09:43:59 UTC
Review of attachment 313606 [details] [review]:

Pushed to master with hash 7ea7ee377. Please mark the bug resolved.
Comment 8 Florian Müllner 2015-10-18 12:11:30 UTC
(In reply to Kunaal Jain from comment #7)
> Please mark the bug resolved.

Sure, but you should be able to do that yourself? (Also git-bz FTW: "git bz push origin HEAD:master" or similar to push+comment+close)
Comment 9 Kunaal Jain 2015-10-18 14:08:08 UTC
(In reply to Florian Müllner from comment #8)
> Sure, but you should be able to do that yourself?

I think, I don't have permissions to close bugs on bugzilla. Do they need to be granted? 

> (Also git-bz FTW: "git bz
> push origin HEAD:master" or similar to push+comment+close)

I should start using this plugin.