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

Allow the user to set highlight without %nick% #103

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

omnidan
Copy link
Contributor

@omnidan omnidan commented Jun 19, 2014

My nick is dan on some networks - including german networks. dann is a common word in german so I get push notifications A LOT because %nick% highlights on *dan*.

I made %nick% the default highlight value and now the user can change it without having %nick% part of the value. This means I can do _%nick% _%nick%: or something like that. IMO this is a nicer way of handling this anyway - what else are default values for?

removed push_back("%nick%"), so the user can set his own highlight without having %nick% in it
@kuzetsa
Copy link

kuzetsa commented Jun 24, 2014

This issue affects me as well, thanks for the patch & I might build your version even if the pull request doesn't get accepted 👍

@amyreese amyreese added this to the 2.0: Python milestone Jul 30, 2014
@amyreese
Copy link
Owner

I agree with the goal, but my biggest concern is how to handle existing users. There's currently no mechanism that would allow new users to get the new default, while also allowing existing users to have the old behavior.

I will consider the best way to implement this with 2.0.

@omnidan
Copy link
Contributor Author

omnidan commented Jul 30, 2014

even with not having %nick% the default it's still not an optimal solution. You need to check for things like "dan:", "hi dan" and "dan", but not "dann", "danach" or "qwopejdanqiwe".

@amyreese
Copy link
Owner

Good point.

@omnidan
Copy link
Contributor Author

omnidan commented Jul 30, 2014

Maybe do it the same way popular clients do it?

@fawaf
Copy link

fawaf commented Apr 23, 2015

also +1

@knu
Copy link
Contributor

knu commented Aug 28, 2015

Would setting _dan list other keywords here -dan to highlight help?

@knu
Copy link
Contributor

knu commented Aug 28, 2015

My understanding is that %nick% is automatically appended to the list, so I think ending your list with -%nick% will nullify it.

@amyreese
Copy link
Owner

Can you update the readme to document the network_blacklist config?

@omnidan
Copy link
Contributor Author

omnidan commented Sep 21, 2015

@jreese I already did that 😱

See: 7fdf468

@amyreese
Copy link
Owner

Yeah, just realized this pull request includes parts that were already merged elsewhere. Which commits still need to be reviewed/merged? Just the first %nick% change?

@omnidan
Copy link
Contributor Author

omnidan commented Sep 21, 2015

@jreese IIRC this removes the part that auto-adds %nick% to the list and makes it a default parameter instead (so you can override that setting)

@dgw dgw removed this from the 2.0: Python milestone May 21, 2019
@dgw
Copy link
Collaborator

dgw commented May 21, 2019

@omnidan Can you clean up this PR branch so only the relevant commit(s) will be merged?

I'd be happy to do it myself, but 1) this PR is so old it predates the GitHub feature allowing maintainers to edit the source branch, and 2) even if that option was enabled, I don't really want to mess with someone else's master without permission. 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants