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

feature: /ignore command #19

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

feature: /ignore command #19

wants to merge 3 commits into from

Conversation

nogaems
Copy link
Contributor

@nogaems nogaems commented Dec 23, 2018

This code adds /ignore command which is not buffer specific and can be used from any tox plugin's buffer.

 [tox]  /ignore  list
                 add <name>
                 del <name>

 ommit messages from people with certain nicks

 list: show the list of ignores
 add: add a nick to the list
 del: delete a nick from the list

When you add a nickname to the ignore list using this command it will filter all the messages in group chats you've joined and show only messages from people not it the list.
It indicates muted people in the nicklist of a group chat with "-" prefix (like "devoice" in IRC) and refreshes this prefix after any changes on the ignore list or changing names of peers in groups.
The list of ignores is preserved across restarts by writing it to the file specified in options (tox.profile.<profile_name>.ignore_file) and it can be manually saved by issuing /save command.

Also I've reverted "clang-format" commit since it leads to multiple merge conflicts and it's way easier to apply it again after merging this branch.
And there's a little fix for #18 that doesn't worth a separate pull request.

Copy link
Owner

@haavard haavard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks sweet! My main concern is the storage format for the ignore list. WeeChat's IRC plugin stores its ignores in irc.conf in a format like this:

[ignore]
ignore = *;*;^foo$
ignore = *;*;^bar$

I think I prefer something like this over a bespoke file that lives outside of WeeChat's configuration "ecosystem". I'm also not sure what happens if someone puts a newline in their nickname; this might break the ignore list on disk as it is right now.

I also think this could be made more powerful by supporting ignoring a Tox ID instead of just a nick. Even if this isn't implemented in this iteration, designing the /ignore command to be extensible for this in the future seems like a good idea.

src/twc-tfer.c Outdated
@@ -160,7 +160,8 @@ twc_tfer_update_downloading_path(struct t_twc_profile *profile)
free(bad_path);
weechat_mkdir_parents(full_path, 0755);
}
free(profile->tfer->downloading_path);
if (profile->tfer->downloading_path)
free(profile->tfer->downloading_path);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary, as calling free(3) on NULL is a no-op.

CMakeLists.txt Outdated
@@ -64,4 +65,3 @@ endif()
set(PLUGIN_PATH "lib/weechat/plugins" CACHE PATH
"Path to install the plugin binary to.")
install(TARGETS tox DESTINATION "${PLUGIN_PATH}")

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary whitespace change.

@nogaems
Copy link
Contributor Author

nogaems commented Dec 25, 2018

Hi @haavard
Well, I actually share your concern about this ad-hoc solution for the storage of nicknames. I got to take a look deeper in weechat documentation and how is this feature implemented in IRC /ignore command, so I think I'll get this done soon.

supporting ignoring a Tox ID instead of just a nick

Unfortunately, it is impossible since uint32_t peer_number you get from callbacks is just an index in tox library representation of participants of a group chat. It's not deriving from a Tox ID and the same person can have different uint32_t peer_number in several chats. This is a part of Tox protocol specification and this is not going to be changed, so it's just as planned.
And by the way, the lacking of persistent group chats is the reason why this feature cannot be implemented "per group chat" and not globally, because there is nothing you can identify a group chat by across restarts of a client. In c-toxcore they always say that it's almost done and going to come out soon, but it already lasts for more than a year and it's better not to wait for it any soon.

@haavard
Copy link
Owner

haavard commented Dec 26, 2018

Hi @haavard
Well, I actually share your concern about this ad-hoc solution for the storage of nicknames. I got to take a look deeper in weechat documentation and how is this feature implemented in IRC /ignore command, so I think I'll get this done soon.

Great! For these things I usually just dive into the WeeChat source code to get inspiration and see how the APIs are used there.

supporting ignoring a Tox ID instead of just a nick

Unfortunately, it is impossible since uint32_t peer_number you get from callbacks is just an index in tox library representation of participants of a group chat. It's not deriving from a Tox ID and the same person can have different uint32_t peer_number in several chats. This is a part of Tox protocol specification and this is not going to be changed, so it's just as planned.

Can't you pretty easily get the Tox ID (public key, technically) with tox_conference_peer_get_public_key?

@nogaems
Copy link
Contributor Author

nogaems commented Mar 27, 2019

Hi, @haavard
I've changed a lot of things here according to your notes.
First off, I've managed to store ignores list in tox.conf such as the irc plugin does. Now it stores profile-specific options in [ignore] section and loads up the values on startup. All the ignores get dumped to the file on /exit or /save command.
Also as long as we have #21 the ingore has been changed to work with Tox IDs, now you can put in the ignore list a substring of hex representation of a Tox ID and the filtering is going to work using it instead of just presented nicknames that don't identify anything.
This has to be merged only after and if #21 gets merged.

@nogaems nogaems changed the title Implement /ignore feature feature: /ignore command Mar 27, 2019
Like it's already implemented in irc plugin, this commit adds the /ignore command. Now you can filter messages in group chats to not be shown, based on a part of Tox ID. The ignore list of IDs is manageable in this way:
[tox]  /ignore  list
                add <ID part>
                del <ID part>

ommit messages from people with certain Tox IDs

list: show the list of ignores
add: add an ID to the list
del: delete an ID from the list
Even though it's a pretty rare case to happen, it's still possible to occure that there are two+ peers in a group chat with the same first 2 bytes of the ID. Since we're using 2 (that means 4 character length) as a minimal possible value for the shortened ID form, I've changed the ignore list storage in the way that you can now store as many bytes as you want regardless of the option `tox.look.short_id_size`. If the ID collision  happens, you can increase the value of that option and you don't have to change your ignore list.
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.

2 participants