Fix #2275, ref #2229

- Refactor the message handlers to be more readable
- Add a group_corrections tab-specific option (#2229)
- Fix issues with /correct in private tabs and conversation tabs

5 files changed, 114 insertions, 90 deletions

diff --git a/src/common.py b/src/common.py
index 16fcfba5..987c93a9 100644
--- a/src/common.py
+++ b/src/common.py
@@ -196,6 +196,31 @@ def datetime_tuple(timestamp):
     ret -= dst
     return ret
 
+def find_delayed_tag(message):
+    """
+    Check if a message is delayed or not.
+
+    :param sleekxmpp.Message message: The message to check.
+    :return: A tuple containing (True, the datetime) or (False, None)
+    :rtype: :py:class:`tuple`
+    """
+
+    delay_tag = message.find('{urn:xmpp:delay}delay')
+    if delay_tag is not None:
+        delayed = True
+        date = datetime_tuple(delay_tag.attrib['stamp'])
+    else:
+        # We support the OLD and deprecated XEP: http://xmpp.org/extensions/xep-0091.html
+        # But it sucks, please, Jabber servers, don't do this :(
+        delay_tag = message.find('{jabber:x:delay}x')
+        if delay_tag is not None:
+            delayed = True
+            date = common.datetime_tuple(delay_tag.attrib['stamp'])
+        else:
+            delayed = False
+            date = None
+    return (delayed, date)
+
 def shell_split(st):
     """
     Split a string correctly according to the quotes
diff --git a/src/core.py b/src/core.py
index 7c9c25e7..97470b48 100644
--- a/src/core.py
+++ b/src/core.py
@@ -2695,16 +2695,11 @@ class Core(object):
         if isinstance(conversation, tabs.DynamicConversationTab):
             conversation.lock(jid.resource)
         conversation.nick = remote_nick
-        delay_tag = message.find('{urn:xmpp:delay}delay')
-        if delay_tag is not None:
-            delayed = True
-            date = common.datetime_tuple(delay_tag.attrib['stamp'])
-        else:
-            delayed = False
-            date = None
+        delayed, date = common.find_delayed_tag(message)
         replaced_id = message['replace']['id']
         replaced = False
-        if replaced_id is not '':
+        if replaced_id is not '' and (config.get_by_tabname(
+            'group_corrections', 'true', jid.bare).lower() != 'false'):
             try:
                 conversation.modify_message(body, replaced_id, message['id'])
                 replaced = True
@@ -2878,59 +2873,54 @@ class Core(object):
         """
         if message['subject']:
             return
-        delay_tag = message.find('{urn:xmpp:delay}delay')
-        if delay_tag is not None:
-            delayed = True
-            date = common.datetime_tuple(delay_tag.attrib['stamp'])
-        else:
-            # We support the OLD and deprecated XEP: http://xmpp.org/extensions/xep-0091.html
-            # But it sucks, please, Jabber servers, don't do this :(
-            delay_tag = message.find('{jabber:x:delay}x')
-            if delay_tag is not None:
-                delayed = True
-                date = common.datetime_tuple(delay_tag.attrib['stamp'])
-            else:
-                delayed = False
-                date = None
-        nick_from = message['mucnick']
         room_from = message.getMucroom()
+
         if message['type'] == 'error': # Check if it's an error
             return self.room_error(message, room_from)
+
         tab = self.get_tab_by_name(room_from, tabs.MucTab)
-        old_state = tab.state
         if not tab:
             self.information(_("message received for a non-existing room: %s") % (room_from))
             return
+
+        nick_from = message['mucnick']
         user = tab.get_user_by_name(nick_from)
         if user and user in tab.ignores:
             return
+
         self.events.trigger('muc_msg', message, tab)
         body = xhtml.get_body_from_message_stanza(message)
-        if body:
-            date = date if delayed == True else None
-            replaced_id = message['replace']['id']
-            replaced = False
-            if replaced_id is not '':
-                try:
-                    if tab.modify_message(body, replaced_id, message['id'], date, nick_from, user):
-                        self.events.trigger('highlight', message, tab)
-                    replaced = True
-                except CorrectionError:
-                    pass
-            if not replaced and tab.add_message(body, date, nick_from, history=True if date else False, identifier=message['id']):
-                self.events.trigger('highlight', message, tab)
-            if tab is self.current_tab():
-                tab.text_win.refresh()
-                tab.info_header.refresh(tab, tab.text_win)
-                tab.input.refresh()
-                self.doupdate()
-            elif tab.state != old_state:
-                self.refresh_tab_win()
-                self.current_tab().input.refresh()
-                self.doupdate()
-            if 'message' in config.get('beep_on', 'highlight private').split():
-                if config.get_by_tabname('disable_beep', 'false', room_from, False).lower() != 'true':
-                    curses.beep()
+        if not body:
+            return
+
+        old_state = tab.state
+        delayed, date = common.find_delayed_tag(message)
+        replaced_id = message['replace']['id']
+        replaced = False
+        if replaced_id is not '' and (config.get_by_tabname(
+            'group_corrections', 'true', message['from'].bare).lower() != 'false'):
+            try:
+                if tab.modify_message(body, replaced_id, message['id'], date, nick_from, user):
+                    self.events.trigger('highlight', message, tab)
+                replaced = True
+            except CorrectionError:
+                pass
+        if not replaced and tab.add_message(body, date, nick_from, history=delayed, identifier=message['id']):
+            self.events.trigger('highlight', message, tab)
+
+        if tab is self.current_tab():
+            tab.text_win.refresh()
+            tab.info_header.refresh(tab, tab.text_win)
+            tab.input.refresh()
+            self.doupdate()
+        elif tab.state != old_state:
+            self.refresh_tab_win()
+            self.current_tab().input.refresh()
+            self.doupdate()
+
+        if 'message' in config.get('beep_on', 'highlight private').split():
+            if config.get_by_tabname('disable_beep', 'false', room_from, False).lower() != 'true':
+                curses.beep()
 
     def on_groupchat_private_message(self, message):
         """
@@ -2957,8 +2947,9 @@ class Core(object):
             return
         replaced_id = message['replace']['id']
         replaced = False
-        if replaced_id is not '':
-            user = self.get_tab_by_name(room_from, tabs.MucTab).get_user_by_name(nick_from),
+        user = tab.parent_muc.get_user_by_name(nick_from)
+        if replaced_id is not '' and (config.get_by_tabname(
+            'group_corrections', 'true', room_from).lower() != 'false'):
             try:
                 tab.modify_message(body, replaced_id, message['id'], user=user)
                 replaced = True
@@ -2966,7 +2957,7 @@ class Core(object):
                 pass
         if not replaced:
             tab.add_message(body, time=None, nickname=nick_from,
-                            forced_user=self.get_tab_by_name(room_from, tabs.MucTab).get_user_by_name(nick_from),
+                            forced_user=user,
                             identifier=message['id'])
         if tab.remote_wants_chatstates is None:
             if message['chat_state']:
diff --git a/src/logger.py b/src/logger.py
index 6b56738e..8b4b1d3c 100644
--- a/src/logger.py
+++ b/src/logger.py
@@ -97,7 +97,7 @@ class Logger(object):
         logs = reads[-nb:]
         return logs
 
-    def log_message(self, jid, nick, msg):
+    def log_message(self, jid, nick, msg, date=None):
         """
         log the message in the appropriate jid's file
         """
@@ -109,10 +109,14 @@ class Logger(object):
             return
         try:
             msg = clean_text(msg)
+            if date is None:
+                str_time = datetime.now().strftime('%d-%m-%y [%H:%M:%S] ')
+            else:
+                str_time = date.strftime('%d-%m-%y [%H:%M:%S] ')
             if nick:
-                fd.write(datetime.now().strftime('%d-%m-%y [%H:%M:%S] ')+nick+': '+msg+'\n')
+                fd.write(''.join((str_time, nick, ': ', msg, '\n')))
             else:
-                fd.write(datetime.now().strftime('%d-%m-%y [%H:%M:%S] ')+'* '+msg+'\n')
+                fd.write(''.join((str_time,  '* ', msg, '\n')))
         except IOError:
             pass
         else:
diff --git a/src/tabs.py b/src/tabs.py
index 70247ab2..5d019bd1 100644
--- a/src/tabs.py
+++ b/src/tabs.py
@@ -48,7 +48,7 @@ from sleekxmpp.xmlstream.handler import Callback
 from config import config
 from roster import RosterGroup, roster
 from contact import Contact, Resource
-from text_buffer import TextBuffer
+from text_buffer import TextBuffer, CorrectionError
 from user import User
 from os import getenv, path
 from logger import logger
@@ -477,13 +477,11 @@ class ChatTab(Tab):
                         user='',
                         str_time=''
                         )
-    def log_message(self, txt, time, nickname):
+    def log_message(self, txt, nickname, time=None):
         """
-        Log the messages in the archives, if it needs
-        to be
+        Log the messages in the archives.
         """
-        if time is None and self.joined:        # don't log the history messages
-            logger.log_message(self.name, nickname, txt)
+        logger.log_message(self.name, nickname, txt, date=time)
 
     def add_message(self, txt, time=None, nickname=None, forced_user=None, nick_color=None, identifier=None):
         self._text_buffer.add_message(txt, time=time,
@@ -494,16 +492,12 @@ class ChatTab(Tab):
                 identifier=identifier)
 
     def modify_message(self, txt, old_id, new_id, user=None):
-        self.log_message(txt, time, self.name)
-        try:
-            message = self._text_buffer.modify_message(txt, old_id, new_id, time=time, user=user)
-        except CorrectionError as e:
-            self.core.information("%s" % (e,), 'Error')
-        else:
-            if message:
-                self.text_win.modify_message(old_id, message)
-                self.core.refresh_window()
-                return True
+        self.log_message(txt, self.name)
+        message = self._text_buffer.modify_message(txt, old_id, new_id, time=time, user=user)
+        if message:
+            self.text_win.modify_message(old_id, message)
+            self.core.refresh_window()
+            return True
         return False
 
     def last_words_completion(self):
@@ -1696,7 +1690,7 @@ class MucTab(ChatTab):
         """
         return self.topic.replace('\n', '|')
 
-    def log_message(self, txt, time, nickname):
+    def log_message(self, txt, nickname, time=None):
         """
         Log the messages in the archives, if it needs
         to be
@@ -1745,7 +1739,7 @@ class MucTab(ChatTab):
         in the room anymore
         Return True if the message highlighted us. False otherwise.
         """
-        self.log_message(txt, time, nickname)
+        self.log_message(txt, nickname, time=time)
         user = self.get_user_by_name(nickname) if nickname is not None else None
         if user:
             user.set_last_talked(datetime.now())
@@ -1768,17 +1762,13 @@ class MucTab(ChatTab):
         return highlight
 
     def modify_message(self, txt, old_id, new_id, time=None, nickname=None, user=None):
-        self.log_message(txt, time, nickname)
+        self.log_message(txt, nickname, time=time)
         highlight = self.do_highlight(txt, time, nickname)
-        try:
-            message = self._text_buffer.modify_message(txt, old_id, new_id, highlight=highlight, time=time, user=user)
-        except CorrectionError as e:
-            self.core.information("%s" % (e,), 'Error')
-        else:
-            if message:
-                self.text_win.modify_message(old_id, message)
-                self.core.refresh_window()
-                return highlight
+        message = self._text_buffer.modify_message(txt, old_id, new_id, highlight=highlight, time=time, user=user)
+        if message:
+            self.text_win.modify_message(old_id, message)
+            self.core.refresh_window()
+            return highlight
         return False
 
     def matching_names(self):
@@ -1859,13 +1849,21 @@ class PrivateTab(ChatTab):
         # This lets a plugin insert \x19xxx} colors, that will
         # be converted in xhtml.
         self.core.events.trigger('private_say', msg, self)
+        user = self.parent_muc.get_user_by_name(self.own_nick)
+        replaced = False
         if correct:
             msg['replace']['id'] = self.last_sent_message['id']
-            self.modify_message(line, self.last_sent_message['id'], msg['id'])
-        else:
+            if config.get_by_tabname('group_corrections', 'true', self.get_name()).lower() != 'false':
+                try:
+                    self.modify_message(line, self.last_sent_message['id'], msg['id'], user=user)
+                    replaced = True
+                except:
+                    pass
+
+        if not replaced:
             self.add_message(msg['body'],
                     nickname=self.core.own_nick or self.own_nick,
-                    forced_user=self.parent_muc.get_user_by_name(self.own_nick),
+                    forced_user=user,
                     nick_color=get_theme().COLOR_OWN_NICK,
                     identifier=msg['id'])
         if msg['body'].find('\x19') != -1:
@@ -3012,10 +3010,16 @@ class ConversationTab(ChatTab):
         # This lets a plugin insert \x19xxx} colors, that will
         # be converted in xhtml.
         self.core.events.trigger('conversation_say', msg, self)
+        replaced = False
         if correct:
             msg['replace']['id'] = self.last_sent_message['id']
-            self.modify_message(line, self.last_sent_message['id'], msg['id'])
-        else:
+            if config.get_by_tabname('group_corrections', 'true', self.get_name()).lower() != 'false':
+                try:
+                    self.modify_message(line, self.last_sent_message['id'], msg['id'])
+                    replaced = True
+                except:
+                    pass
+        if not replaced:
             self.add_message(msg['body'],
                     nickname=self.core.own_nick,
                     nick_color=get_theme().COLOR_OWN_NICK,
diff --git a/src/text_buffer.py b/src/text_buffer.py
index 3f24760d..62489af8 100644
--- a/src/text_buffer.py
+++ b/src/text_buffer.py
@@ -110,9 +110,9 @@ class TextBuffer(object):
             msg = self.messages[i]
             if msg.identifier == old_id:
                 if msg.user and msg.user is not user:
-                    raise CorrectionError("wrong user")
+                    raise CorrectionError("Different users")
                 elif len(msg.str_time) > 8: # ugly
-                    raise CorrectionError("delayed message")
+                    raise CorrectionError("Delayed message")
                 message = self.make_message(txt, time if time else msg.time, msg.nickname, msg.nick_color, None, msg.user, new_id, highlight=highlight, old_message=msg, revisions=msg.revisions + 1)
                 self.messages[i] = message
                 log.debug('Replacing message %s with %s.', old_id, new_id)