muc: remove non-deterministic nick colors

9 files changed, 42 insertions, 118 deletions

diff --git a/data/default_config.cfg b/data/default_config.cfg
index a2e4aaaf..4405164f 100644
--- a/data/default_config.cfg
+++ b/data/default_config.cfg
@@ -404,9 +404,6 @@ use_bookmarks_method =
 # possible values: desc, asc
 #user_list_sort = desc
 
-# If the chatroom nicks should receive a fixed color based on their text or not
-#deterministic_nick_colors = true
-
 # If _nick, nick_, _nick_, nick__ etc. should have the same color as nick
 #nick_color_aliases = true
 
diff --git a/doc/source/commands.rst b/doc/source/commands.rst
index e4f90560..2a179b8d 100644
--- a/doc/source/commands.rst
+++ b/doc/source/commands.rst
@@ -343,8 +343,7 @@ MultiUserChat tab commands
         are considered identical if they only differ by the presence of one
         ore more **_** character at the beginning or the end. For example
         _Foo and Foo___ are considered aliases of the nick Foo) will then
-        always have the specified color, in all MultiUserChat tabs.  This is
-        true whatever the value of **deterministic_nick_colors** is.
+        always have the specified color, in all MultiUserChat tabs.
 
         Use the completion to get a list of all the available color values.
         Use the special color **unset** to remove the attributed color on
@@ -428,12 +427,10 @@ MultiUserChat tab commands
         Change your nickname in the current room.
 
     /recolor
-        **Usage:** ``/recolor [random]``
+        **Usage:** ``/recolor``
 
-        Re-assign a color to all the participants in the current
-        room, based on the last time they talked. Use this if the participants
-        currently talking have too many identical colors. If a random argument is
-        given, the participants will be shuffled before they are assigned a color.
+        Re-assign a color to all the participants in the current room,
+        if the theme has changed.
 
     /cycle
         **Usage:** ``/cycle [message]``
diff --git a/doc/source/configuration.rst b/doc/source/configuration.rst
index fc6c9bfd..8b364961 100644
--- a/doc/source/configuration.rst
+++ b/doc/source/configuration.rst
@@ -512,23 +512,14 @@ or the way messages are displayed.
         bottom in the list, if set to ``asc``, they will be displayed from
         bottom to top.
 
-    deterministic_nick_colors
-
-        **Default value:** ``true``
-
-        Use a deterministic algorithm to choose the user colors in chatrooms if
-        set to ``true``. Otherwise the colors will be picked randomly.
-
-        The value of this option affects the behavior of :term:`/recolor`.
-
     nick_color_aliases
 
         **Default value:** ``true``
 
-	Automatically search for color of nick aliases. For example, if nick is
-	set to red, _nick, nick\_, _nick_, nick\__ etc. will have the same color.
-	Aliases colors are checked first, so that it is still possible to have
-	different colors for nick\_ and nick.
+        Automatically search for color of nick aliases. For example, if nick is
+        set to red, _nick, nick\_, _nick_, nick\__ etc. will have the same color.
+        Aliases colors are checked first, so that it is still possible to have
+        different colors for nick\_ and nick.
 
     vertical_tab_list_size
 
diff --git a/poezio/config.py b/poezio/config.py
index 00d0b2f7..2bd63274 100644
--- a/poezio/config.py
+++ b/poezio/config.py
@@ -49,7 +49,6 @@ DEFAULT_CONFIG = {
         'custom_port': '',
         'default_nick': '',
         'default_muc_service': '',
-        'deterministic_nick_colors': True,
         'device_id': '',
         'nick_color_aliases': True,
         'display_activity_notifications': False,
diff --git a/poezio/core/core.py b/poezio/core/core.py
index efab8810..08ad8799 100644
--- a/poezio/core/core.py
+++ b/poezio/core/core.py
@@ -329,7 +329,6 @@ class Core:
             ('connection_check_interval', self.xmpp.set_keepalive_values),
             ('connection_timeout_delay', self.xmpp.set_keepalive_values),
             ('create_gaps', self.on_gaps_config_change),
-            ('deterministic_nick_colors', self.on_nick_determinism_changed),
             ('enable_carbons', self.on_carbons_switch),
             ('enable_vertical_tab_list',
              self.on_vertical_tab_list_config_change),
@@ -441,14 +440,6 @@ class Core:
         """
         self.xmpp.password = value
 
-    def on_nick_determinism_changed(self, option, value):
-        """If we change the value to true, we call /recolor on all the MucTabs, to
-        make the current nick colors reflect their deterministic value.
-        """
-        if value.lower() == "true":
-            for tab in self.get_tabs(tabs.MucTab):
-                tab.command_recolor('')
-
     def on_carbons_switch(self, option, value):
         """Whenever the user enables or disables carbons using /set, we should
         inform the server immediately, this way we do not require a restart
diff --git a/poezio/mam.py b/poezio/mam.py
index b40ddc4d..468d12e7 100644
--- a/poezio/mam.py
+++ b/poezio/mam.py
@@ -48,7 +48,6 @@ def make_line(
         time: datetime,
         jid: JID,
         identifier: str = '',
-        deterministic: bool = True,
     ) -> Message:
     """Adds a textual entry in the TextBuffer"""
 
@@ -59,22 +58,17 @@ def make_line(
     if isinstance(tab, tabs.MucTab):
         nick = jid.resource
         user = tab.get_user_by_name(nick)
-        if deterministic:
-            if user:
-                color = user.color
-            else:
-                theme = get_theme()
-                if theme.ccg_palette:
-                    fg_color = colors.ccg_text_to_color(theme.ccg_palette, nick)
-                    color = fg_color, -1
-                else:
-                    mod = len(theme.LIST_COLOR_NICKNAMES)
-                    nick_pos = int(md5(nick.encode('utf-8')).hexdigest(), 16) % mod
-                    color = theme.LIST_COLOR_NICKNAMES[nick_pos]
+        if user:
+            color = user.color
         else:
-            color = random.choice(list(xhtml.colors))
-            color = xhtml.colors.get(color)
-            color = (color, -1)
+            theme = get_theme()
+            if theme.ccg_palette:
+                fg_color = colors.ccg_text_to_color(theme.ccg_palette, nick)
+                color = fg_color, -1
+            else:
+                mod = len(theme.LIST_COLOR_NICKNAMES)
+                nick_pos = int(md5(nick.encode('utf-8')).hexdigest(), 16) % mod
+                color = theme.LIST_COLOR_NICKNAMES[nick_pos]
     else:
         if jid.bare == tab.core.xmpp.boundjid.bare:
             nick = tab.core.own_nick
@@ -144,21 +138,16 @@ async def retrieve_messages(tab: tabs.ChatTab,
                             results: AsyncIterable[SMessage],
                             amount: int = 100) -> List[BaseMessage]:
     """Run the MAM query and put messages in order"""
-    text_buffer = tab._text_buffer
     msg_count = 0
     msgs = []
     to_add = []
-    deterministic = config.get_by_tabname(
-        'deterministic_nick_colors',
-        tab.jid.bare
-    )
     try:
         async for rsm in results:
             for msg in rsm['mam']['results']:
                 if msg['mam_result']['forwarded']['stanza'] \
                         .xml.find('{%s}%s' % ('jabber:client', 'body')) is not None:
                     args = _parse_message(msg)
-                    msgs.append(make_line(tab, deterministic=deterministic, **args))
+                    msgs.append(make_line(tab, **args))
             for msg in reversed(msgs):
                 to_add.append(msg)
                 msg_count += 1
diff --git a/poezio/tabs/muctab.py b/poezio/tabs/muctab.py
index ce38b8ac..2c06a52d 100644
--- a/poezio/tabs/muctab.py
+++ b/poezio/tabs/muctab.py
@@ -380,37 +380,15 @@ class MucTab(ChatTab):
         )
 
     @refresh_wrapper.always
-    def recolor(self, random_colors: bool = False) -> None:
+    def recolor(self) -> None:
         """Recolor the current MUC users"""
-        deterministic = config.get_by_tabname('deterministic_nick_colors',
-                                              self.jid.bare)
-        if deterministic:
-            for user in self.users:
-                if user is self.own_user:
-                    continue
-                color = self.search_for_color(user.nick)
-                if color != '':
-                    continue
-                user.set_deterministic_color()
-            return
-        # Sort the user list by last talked, to avoid color conflicts
-        # on active participants
-        sorted_users = sorted(self.users, key=COMPARE_USERS_LAST_TALKED, reverse=True)
-        full_sorted_users = sorted_users[:]
-        # search our own user, to remove it from the list
-        # Also remove users whose color is fixed
-        for user in full_sorted_users:
-            color = self.search_for_color(user.nick)
+        for user in self.users:
             if user is self.own_user:
-                sorted_users.remove(user)
-            elif color != '':
-                sorted_users.remove(user)
-                user.change_color(color, deterministic)
-        colors = list(get_theme().LIST_COLOR_NICKNAMES)
-        if random_colors:
-            random.shuffle(colors)
-        for i, user in enumerate(sorted_users):
-            user.color = colors[i % len(colors)]
+                continue
+            color = self.search_for_color(user.nick)
+            if color != '':
+                continue
+            user.set_deterministic_color()
         self.text_win.rebuild_everything(self._text_buffer)
 
     @refresh_wrapper.conditional
@@ -527,16 +505,13 @@ class MucTab(ChatTab):
         """
         Batch-process all the initial presences
         """
-        deterministic = config.get_by_tabname('deterministic_nick_colors',
-                                              self.jid.bare)
-
         for stanza in self.presence_buffer:
             try:
-                self.handle_presence_unjoined(stanza, deterministic)
+                self.handle_presence_unjoined(stanza)
             except PresenceError:
                 self.core.room_error(stanza, stanza['from'].bare)
         self.presence_buffer = []
-        self.handle_presence_unjoined(last_presence, deterministic, own)
+        self.handle_presence_unjoined(last_presence, own)
         self.users.sort()
         # Enable the self ping event, to regularly check if we
         # are still in the room.
@@ -547,7 +522,7 @@ class MucTab(ChatTab):
             self.core.tabs.current_tab.refresh_input()
             self.core.doupdate()
 
-    def handle_presence_unjoined(self, presence: Presence, deterministic: bool, own: bool = False) -> None:
+    def handle_presence_unjoined(self, presence: Presence, own: bool = False) -> None:
         """
         Presence received while we are not in the room (before code=110)
         """
@@ -560,7 +535,7 @@ class MucTab(ChatTab):
             return
         user_color = self.search_for_color(from_nick)
         new_user = User(from_nick, affiliation, show, status, role, jid,
-                        deterministic, user_color)
+                        user_color)
         self.users.append(new_user)
         self.core.events.trigger('muc_join', presence, self)
         if own:
@@ -710,10 +685,8 @@ class MucTab(ChatTab):
         """
         When a new user joins the groupchat
         """
-        deterministic = config.get_by_tabname('deterministic_nick_colors',
-                                              self.jid.bare)
         user = User(from_nick, affiliation, show, status, role, jid,
-                    deterministic, color)
+                    color)
         bisect.insort_left(self.users, user)
         hide_exit_join = config.get_by_tabname('hide_exit_join',
                                                self.general_jid)
@@ -763,11 +736,8 @@ class MucTab(ChatTab):
             user.change_nick(new_nick)
         else:
             user.change_nick(new_nick)
-            deterministic = config.get_by_tabname('deterministic_nick_colors',
-                                                  self.jid.bare)
             color = config.get_by_tabname(new_nick, 'muc_colors') or None
-            if color or deterministic:
-                user.change_color(color, deterministic)
+            user.change_color(color)
         self.users.remove(user)
         bisect.insort_left(self.users, user)
 
@@ -2169,12 +2139,11 @@ class MucTab(ChatTab):
             'func':
             self.command_recolor,
             'usage':
-            '[random]',
-            'desc': ('Re-assign a color to all participants of the'
-                     ' current room, based on the last time they talked.'
-                     ' Use this if the participants currently talking '
-                     'have too many identical colors. Use /recolor random'
-                     ' for a non-deterministic result.'),
+            '',
+            'desc': (
+                'Re-assign a color to all participants of the room '
+                'if the theme has changed.'
+            ),
             'shortdesc':
             'Change the nicks colors.',
             'completion':
diff --git a/poezio/user.py b/poezio/user.py
index 3724d229..858c6d0e 100644
--- a/poezio/user.py
+++ b/poezio/user.py
@@ -38,7 +38,6 @@ class User:
                  status: str,
                  role: str,
                  jid: JID,
-                 deterministic=True,
                  color='') -> None:
         # The oldest possible time
         self.last_talked: datetime = datetime(1, 1, 1)
@@ -48,12 +47,9 @@ class User:
         self.chatstate: Optional[str] = None
         self.color: Tuple[int, int] = (1, 1)
         if color != '':
-            self.change_color(color, deterministic)
+            self.change_color(color)
         else:
-            if deterministic:
-                self.set_deterministic_color()
-            else:
-                self.color = choice(get_theme().LIST_COLOR_NICKNAMES)
+            self.set_deterministic_color()
 
     def set_deterministic_color(self) -> None:
         theme = get_theme()
@@ -82,14 +78,10 @@ class User:
     def change_nick(self, nick: str):
         self.nick = nick
 
-    def change_color(self, color_name: Optional[str], deterministic=False):
+    def change_color(self, color_name: Optional[str]):
         color = xhtml.colors.get(color_name or '')
         if color is None:
-            if deterministic:
-                self.set_deterministic_color()
-            else:
-                log.error('Unknown color "%s"', color_name)
-                self.color = choice(get_theme().LIST_COLOR_NICKNAMES)
+            self.set_deterministic_color()
         else:
             self.color = (color, -1)
 
diff --git a/test/test_user.py b/test/test_user.py
index a462f45b..e3222383 100644
--- a/test/test_user.py
+++ b/test/test_user.py
@@ -17,7 +17,6 @@ def user1():
         'My Status!',
         'moderator',
         JID('foo@muc/nick1'),
-        False,
         'red',
     )
 
@@ -41,5 +40,5 @@ def test_change_nick(user1):
 
 
 def test_change_color(user1):
-    user1.change_color('blue', deterministic=False)
+    user1.change_color('blue')
     assert user1.color == (21, -1)