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

irc_nick_find_color is case sensitive #194

Closed
phy1729 opened this issue Sep 10, 2014 · 6 comments
Closed

irc_nick_find_color is case sensitive #194

phy1729 opened this issue Sep 10, 2014 · 6 comments
Assignees
Labels
bug Unexpected problem or unintended behavior
Milestone

Comments

@phy1729
Copy link
Contributor

phy1729 commented Sep 10, 2014

Since nicks on IRC are not case sensitive, irc_nick_find_color should return the same value regardless of how nickname is capitalized.

@flashcode flashcode added the bug Unexpected problem or unintended behavior label Sep 19, 2014
@flashcode flashcode self-assigned this Aug 23, 2023
@flashcode flashcode added the in progress Someone is working on this issue label Aug 23, 2023
@flashcode flashcode added this to the 4.1.0 milestone Aug 23, 2023
@flashcode
Copy link
Member

flashcode commented Aug 23, 2023

Hi,

Sorry for the 9-year waiting period! 😄

In the meanwhile, the infos "irc_nick_color" and "irc_nick_color_name" have been moved from IRC plugin to core and are now named "nick_color" and "nick_color_name" (see #262).

What I propose is:

  • remove deprecation note on infos irc_nick_color and irc_nick_color_name and use these functions again, in addition to the nick coloring feature in core (infos nick_color and nick_color_name) ;
  • the IRC functions will first convert nick to lower case (following the server CASEMAPPING value), then call the core function ;
  • the format of info changes to: server,nick where server is optional to stay compatible with the scripts still using this info (which is currently deprecated); if server is not given, the CASEMAPPING used is RFC1459 (default in server).

That means for example: weechat.info_get("irc_nick_color", "libera,alice") and weechat.info_get("irc_nick_color", "libera,ALICE") will always return the same color, whereas weechat.info_get("nick_color", "alice") and weechat.info_get("nick_color", "ALICE") could return different colors (if no luck, same color could still be returned for different nicks, according to number of colors in config).

flashcode added a commit that referenced this issue Aug 24, 2023
@flashcode flashcode removed the in progress Someone is working on this issue label Aug 24, 2023
@trygveaa
Copy link
Contributor

trygveaa commented Oct 16, 2023

Sorry, I didn't notice the proposed solution earlier, so this is a bit late, but I think this solution is not ideal. It means that any plugin/script reading nick colors have to check if you are in an irc buffer and request different info depending on that (like you did in colorize_nicks.py). In addition to this being inconvenient, I've seen many scripts using irc specific signals/infos when not necessary so this might happen here as well, which would mean wrong nick colors in non-irc buffers. It also means all scripts reading nick colors need to implement specific support for all plugins/scripts that create buffers (even though irc is currently the only plugin using different nick colors than the default weechat coloring, it goes against the modular design imo).

I think it would be better to have some mechanism for plugins/scripts that creates buffers to override how nick colors are determined, and have the nick_color info take in the buffer pointer and return the correct color for that buffer. Then scripts/plugins that read nick colors could just use the nick_color info with the buffer as a parameter, and not have to care about which plugin/script owns the buffer.

@flashcode
Copy link
Member

Hi,

You're completely right, I was wrong on these changes 😕
So I reopen this issue.

Now we have to repair this, either in a version 4.1.1 or 4.2.0.

There's already a similar feature with a function callback nickcmp_callback in the buffer (used to sort nicklist), but I think this can not be used in scripts.
So we have to take a different approach.
We could have another buffer variable, or local variable, that could be set to the way the nick must be hashed: case sensitive (by default), case insensitive with a range of chars (for IRC plugin).

Then the different info must be adapted to take care of the buffer, if possible in a compatible way.
And maybe the IRC info that I reintroduced could then be removed for good.

I'm thinking about the best solution and will make a proposal here.

Any thoughts?

@flashcode flashcode reopened this Oct 17, 2023
@trygveaa
Copy link
Contributor

There's already a similar feature with a function callback nickcmp_callback in the buffer (used to sort nicklist), but I think this can not be used in scripts.

Ah, this sounds like a very similar thing. Might be inconsistent to use different approaches for these two? I didn't know about nickcmp_callback, but from looking at it now I would think that any plugin/script that needs to set a behavior for nick colors (e.g. case insensitive), would also need to use nickcmp_callback.

The reason I'm thinking that is that if you set colors to be case insensitive, you have multiple potential nicks that equals the same, and if you have that and don't set nickcmp_callback, then the weechat.look.color_nick_offline will not show the correct nicks as online/offline if multiple versions of the nick is used (e.g. if you change nick from nick to Nick in IRC).

So before deciding on the solution, I think we have to consider if both of these, only the color setting, or neither should be available to scripts.

Generally I think it's really great that WeeChat exposes most of the plugin API to scripts so you're able to do almost all of the same things. Many of the more advanced features (e.g. custom config file/sections and hdata) are used by several scripts. I do understand that it takes extra work though, and I don't know if any scripts currently has a use case of these nick options (but always hard to know what scripts want to do in the future).

So I think I'm leaning against doing this the same way as nickcmp_callback and either have neither available for scripts, or find a way to make both available for scripts.

So we have to take a different approach. We could have another buffer variable, or local variable, that could be set to the way the nick must be hashed: case sensitive (by default), case insensitive with a range of chars (for IRC plugin).

When I think a bit more about it, maybe this could replace nickcmp_callback too (or be used as a fallback). As far as I can see, it's currently only used to check if nicks are equal, not the sort order of them, so a normalize option like you suggest here could suffice both for the nick comparisons and color.

@flashcode
Copy link
Member

As mentioned in #2032 (comment), I'll partially revert changes done for this issue, at least in IRC plugin, to compute nick in case sensitive way again (I'll keep infos "nick_color_ignore_case" and "nick_color_name_ignore_case").

@flashcode flashcode modified the milestones: 4.1.0, 4.1.1 Oct 19, 2023
flashcode added a commit that referenced this issue Oct 19, 2023
flashcode added a commit that referenced this issue Oct 19, 2023
@flashcode
Copy link
Member

It's fixed on branches 4.1 and master, please try and report any issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants