From 9d8b56bc0fa9770e05f8cd5d3dfa0bb375e4cc1d Mon Sep 17 00:00:00 2001 From: Marcin Mielniczuk Date: Tue, 25 Sep 2018 16:56:08 +0200 Subject: [PATCH] Refactor groupchat nick auto completion --- gajim/groupchat_control.py | 56 +++++++-------- gajim/gtk/util.py | 86 ++++++++++++++++++++++++ test/no_gui/unit/test_nick_completion.py | 48 +++++++++++++ 3 files changed, 160 insertions(+), 30 deletions(-) create mode 100644 test/no_gui/unit/test_nick_completion.py diff --git a/gajim/groupchat_control.py b/gajim/groupchat_control.py index aefb33764..83beca834 100644 --- a/gajim/groupchat_control.py +++ b/gajim/groupchat_control.py @@ -8,6 +8,7 @@ # Stephan Erb # Copyright (C) 2008 Brendan Taylor # Jonathan Schleifer +# Copyright (C) 2018 Marcin Mielniczuk # # This file is part of Gajim. # @@ -32,6 +33,7 @@ import logging from enum import IntEnum, unique import nbxmpp + from gi.repository import Gtk from gi.repository import Gdk from gi.repository import Pango @@ -75,6 +77,7 @@ from gajim.gtk.add_contact import AddNewContactWindow from gajim.gtk.tooltips import GCTooltip from gajim.gtk.groupchat_config import GroupchatConfig from gajim.gtk.adhoc_commands import CommandWindow +from gajim.gtk.util import NickCompletionGenerator from gajim.gtk.util import get_icon_name from gajim.gtk.util import get_affiliation_surface from gajim.gtk.util import get_builder @@ -317,6 +320,7 @@ class GroupchatControl(ChatControlBase): # sorted list of nicks who mentioned us (last at the end) self.attention_list = [] self.nick_hits = [] + self._nick_completion = NickCompletionGenerator(self.nick) self.last_key_tabs = False self.subject = '' @@ -1433,11 +1437,7 @@ class GroupchatControl(ChatControlBase): other_tags_for_name.append('bold') other_tags_for_text.append('marked') - if contact in self.attention_list: - self.attention_list.remove(contact) - elif len(self.attention_list) > 6: - self.attention_list.pop(0) # remove older - self.attention_list.append(contact) + self._nick_completion.record_message(contact, highlight) if text.startswith('/me ') or text.startswith('/me\n'): other_tags_for_text.append('gc_nickname_color_' + \ @@ -1826,6 +1826,10 @@ class GroupchatControl(ChatControlBase): for role in ('visitor', 'participant', 'moderator'): self.draw_role(role) + def _change_nick(self, new_nick: str) -> None: + self.nick = new_nick + self._nick_completion.change_nick(new_nick) + def _nec_gc_presence_received(self, obj): if obj.room_jid != self.room_jid or obj.conn.name != self.account: return @@ -1945,7 +1949,7 @@ class GroupchatControl(ChatControlBase): elif '303' in obj.status_code: # Someone changed their nick if obj.new_nick == self.new_nick or obj.nick == self.nick: # We changed our nick - self.nick = obj.new_nick + self.change_nick(obj.new_nick) self.new_nick = '' s = _('You are now known as %s') % self.nick else: @@ -1963,8 +1967,7 @@ class GroupchatControl(ChatControlBase): # after that, but that doesn't hurt self.add_contact_to_roster(obj.new_nick, obj.show, role, affiliation, obj.status, obj.real_jid) - if obj.nick in self.attention_list: - self.attention_list.remove(obj.nick) + self._nick_completion.contact_renamed(nick, obj.new_nick) # keep nickname color if obj.nick in self.gc_custom_colors: self.gc_custom_colors[obj.new_nick] = \ @@ -2016,7 +2019,7 @@ class GroupchatControl(ChatControlBase): if not iter_: if '210' in obj.status_code: # Server changed our nick - self.nick = obj.nick + self.change_nick(obj.nick) s = _('You are now known as %s') % nick self.print_conversation(s, 'info', graphics=False) iter_ = self.add_contact_to_roster(obj.nick, obj.show, role, @@ -2080,9 +2083,6 @@ class GroupchatControl(ChatControlBase): right_changed: st = '' - if obj.show == 'offline': - if obj.nick in self.attention_list: - self.attention_list.remove(obj.nick) if obj.show == 'offline' and print_status in ('all', 'in_and_out') \ and (not obj.status_code or '307' not in obj.status_code): st = _('%s has left') % nick_jid @@ -2469,6 +2469,10 @@ class GroupchatControl(ChatControlBase): self.print_conversation(_('%(jid)s has been invited in this room') % {'jid': contact_jid}, graphics=False) + def _jid_not_blocked(self, bare_jid: str) -> bool: + fjid = self.room_jid + '/' + bare_jid + return not helpers.jid_is_blocked(self.account, fjid) + def _on_message_textview_key_press_event(self, widget, event): res = ChatControlBase._on_message_textview_key_press_event(self, widget, event) @@ -2508,29 +2512,21 @@ class GroupchatControl(ChatControlBase): self.nick_hits.append(self.nick_hits[0]) begin = self.nick_hits.pop(0) else: - self.nick_hits = [] # clear the hit list list_nick = app.contacts.get_nick_list(self.account, self.room_jid) - list_nick.sort(key=str.lower) # case-insensitive sort - if begin == '': - # empty message, show lasts nicks that highlighted us first - for nick in self.attention_list: - if nick in list_nick: - list_nick.remove(nick) - list_nick.insert(0, nick) + list_nick = list(filter(self._jid_not_blocked, list_nick)) + + log.debug("Nicks to be considered for autosuggestions: %s", + list_nick) + self.nick_hits = self._nick_completion.generate_suggestions( + nicks=list_nick, beginning=begin) + log.debug("Nicks filtered for autosuggestions: %s", + self.nick_hits) - if self.nick in list_nick: - list_nick.remove(self.nick) # Skip self - for nick in list_nick: - fjid = self.room_jid + '/' + nick - if nick.lower().startswith(begin.lower()) and not \ - helpers.jid_is_blocked(self.account, fjid): - # the word is the beginning of a nick - self.nick_hits.append(nick) if self.nick_hits: if len(splitted_text) < 2 or with_refer_to_nick_char: - # This is the 1st word of the line or no word or we are cycling - # at the beginning, possibly with a space in one nick + # This is the 1st word of the line or no word or we are cycling + # at the beginning, possibly with a space in one nick add = gc_refer_to_nick_char + ' ' else: add = ' ' diff --git a/gajim/gtk/util.py b/gajim/gtk/util.py index ef620a901..790575083 100644 --- a/gajim/gtk/util.py +++ b/gajim/gtk/util.py @@ -1,3 +1,6 @@ +# Copyright (C) 2018 Marcin Mielniczuk +# Copyright (C) 2018 Philipp Hörist +# # This file is part of Gajim. # # Gajim is free software; you can redistribute it and/or modify @@ -42,6 +45,89 @@ if _icon_theme is not None: log = logging.getLogger('gajim.gtk.util') +class NickCompletionGenerator: + def __init__(self, self_nick: str) -> None: + self.nick = self_nick + self.sender_list = [] # type: List[str] + self.attention_list = [] # type: List[str] + + def change_nick(self, new_nick: str) -> None: + self.nick = new_nick + + def record_message(self, contact: str, highlight: bool) -> None: + if contact == self.nick: + return + + log.debug('Recorded a message from %s, highlight; %s', contact, + highlight) + if highlight: + try: + self.attention_list.remove(contact) + except ValueError: + pass + if len(self.attention_list) > 6: + self.attention_list.pop(0) # remove older + self.attention_list.append(contact) + + # TODO implement it in a more efficient way + # Currently it's O(n*m + n*s), where n is the number of participants and + # m is the number of messages processed, s - the number of times the + # suggestions are requested + # + # A better way to do it would be to keep a dict: contact -> timestamp + # with expected O(1) insert, and sort it by timestamps in O(n log n) + # for each suggestion (currently generating the suggestions is O(n)) + # this would give the expected complexity of O(m + s * n log n) + try: + self.sender_list.remove(contact) + except ValueError: + pass + self.sender_list.append(contact) + + def contact_renamed(self, contact_old: str, contact_new: str) -> None: + log.debug('Contact %s renamed to %s', contact_old, contact_new) + for lst in (self.attention_list, self.sender_list): + for idx, contact in enumerate(lst): + if contact == contact_old: + lst[idx] = contact_new + + + def generate_suggestions(self, nicks: List[str], + beginning: str) -> List[str]: + """ + Generate the order of suggested MUC autocompletions + + `nicks` is the list of contacts currently participating in a MUC + `beginning` is the text already typed by the user + """ + def nick_matching(nick: str) -> bool: + return nick != self.nick \ + and nick.lower().startswith(beginning.lower()) + + if beginning == '': + # empty message, so just suggest recent mentions + potential_matches = self.attention_list + else: + # nick partially typed, try completing it + potential_matches = self.sender_list + + potential_matches_set = set(potential_matches) + log.debug('Priority matches: %s', potential_matches_set) + + matches = [n for n in potential_matches if nick_matching(n)] + # the most recent nick is the last one on the list + matches.reverse() + + # handle people who have not posted/mentioned us + other_nicks = [ + n for n in nicks if nick_matching(n) and n not in potential_matches_set + ] + other_nicks.sort(key=str.lower) + log.debug('Other matches: %s', other_nicks) + + return matches + other_nicks + + class Builder: def __init__(self, filename: str, diff --git a/test/no_gui/unit/test_nick_completion.py b/test/no_gui/unit/test_nick_completion.py new file mode 100644 index 000000000..643a09137 --- /dev/null +++ b/test/no_gui/unit/test_nick_completion.py @@ -0,0 +1,48 @@ +import unittest + +from gajim.gtk.util import NickCompletionGenerator + +class Test(unittest.TestCase): + + def test_generate_suggestions(self): + gen = NickCompletionGenerator('meeeee') + + l = ['aaaa', 'meeeee', 'fooo', 'xxxxz', 'xaaaz'] + for n in l: + gen.record_message(n, False) + l2 = ['xxx'] + l + r = gen.generate_suggestions(nicks=l2, beginning='x') + self.assertEqual(r, ['xaaaz', 'xxxxz', 'xxx']) + + r = gen.generate_suggestions( + nicks=l2, + beginning='m' + ) + self.assertEqual(r, []) + + for n in ['xaaaz', 'xxxxz']: + gen.record_message(n, True) + + r = gen.generate_suggestions( + nicks=l2, + beginning='x' + ) + self.assertEqual(r, ['xxxxz', 'xaaaz', 'xxx']) + r = gen.generate_suggestions( + nicks=l2, + beginning='' + ) + self.assertEqual(r, ['xxxxz', 'xaaaz', 'aaaa', 'fooo', 'xxx']) + + l2[1] = 'bbbb' + gen.contact_renamed('aaaa', 'bbbb') + r = gen.generate_suggestions( + nicks=l2, + beginning='' + ) + self.assertEqual(r, ['xxxxz', 'xaaaz', 'bbbb', 'fooo', 'xxx']) + + + +if __name__ == "__main__": + unittest.main()