pazz / alot

Terminal-based Mail User Agent
GNU General Public License v3.0
696 stars 165 forks source link

mark messages as unread by only touching their summary #608

Open josch opened 11 years ago

josch commented 11 years ago

Hi,

I'm now running alot master to make use of the Mail-Followup-To functionality - thanks for adding it! :smiley:

Unfortunately this made me run into another problem. A new behaviour is, that messages in thread view are only marked unread if the cursor is NOT on their summary.

When having big threads I cycle through all the messages using the spacebar and press enter to fold the message so that I can easier scroll to other unread messages in the thread. With this new behaviour I have to cycle through them using spacebar+down+enter because otherwise the messages do not get marked as being read.

This is also annoying when new threads are started and I want to quickly mark them as being read so that they dont show up anymore in my tag:unread buffer.

This all sounds like a minor optimization but in the long run it is not nice for me to have to press more keys for something that in the past took me less.

I tried to add a new configuration option which would mark messages as unread by only touching their summaries in thread view.

diff --git a/alot/buffers.py b/alot/buffers.py
index 12e0dbc..a51119c 100644
--- a/alot/buffers.py
+++ b/alot/buffers.py
@@ -362,8 +362,12 @@ class ThreadBuffer(Buffer):
             mid = msg.get_message_id()
             focus_pos = self.body.get_focus()[1]
             summary_pos = (self.body.get_focus()[1][0], (0,))
-            cursor_on_non_summary = (focus_pos != summary_pos)
-            if cursor_on_non_summary:
+            cursor_on_summary = (focus_pos == summary_pos)
+            unread_on_summary = settings.get('remove_unread_on_summary_touch')
+            # remove the unread flag only if cursor is on a summary and the
+            # config option is set or if the cursor is not on a summary and
+            # the config option is not set
+            if cursor_on_summary == unread_on_summary:
                 if not mid in self._auto_unread_dont_touch_mids:
                     if 'unread' in msg.get_tags():
                         logging.debug('Tbuffer: removing unread')
@@ -384,7 +388,10 @@ class ThreadBuffer(Buffer):
                 if not self._auto_unread_writing and \
                    mid in self._auto_unread_dont_touch_mids:
                     self._auto_unread_dont_touch_mids.remove(mid)
-                logging.debug('Tbuffer: No, cursor on summary')
+                if cursor_on_summary:
+                    logging.debug('Tbuffer: No, cursor on summary')
+                else:
+                    logging.debug('Tbuffer: No, cursor not on summary')
         return self.body.render(size, focus)

     def get_selected_mid(self):
diff --git a/alot/defaults/alot.rc.spec b/alot/defaults/alot.rc.spec
index da15831..6cc795d 100644
--- a/alot/defaults/alot.rc.spec
+++ b/alot/defaults/alot.rc.spec
@@ -4,6 +4,10 @@ ask_subject = boolean(default=True) # ask for subject when compose
 # automatically remove 'unread' tag when focussing messages in thread mode
 auto_remove_unread = boolean(default=True)

+# if 'auto_remove_unread' is set, remove the 'unread' already when focussing
+# the message summary in thread mode
+remove_unread_on_summary_touch = boolean(default=False)
+
 # prompt for initial tags when compose
 compose_ask_tags = boolean(default=False)

But then found that an unexpected side effect was, that unread threads with only a single message inside get opened with that single message being folded instead of it being unfolded. Ideas?

An alternative would be if messages would be marked as unread also if I fold them. Would you like that better than the above proposed change?

pazz commented 11 years ago

why don't you map a command sequence

fold; untag read; move next

?

josch commented 11 years ago

Thanks, I did not know that keybindings could contain multiple commands separated by semicolons. I also didnt find that in the documentation about keybindings?

I now added some keybindings which help me with this issue.

The only remaining problem are threads that only contain a single email. I open it, read the email and close it. I should not have to hit another key to mark that single email as unread, no?

josch commented 11 years ago

Or lets extend the last thing to: any thread where only one email in it is unread.

If I open a thread with only one unread email, then I automatically jump to that email. Why would I have to hit yet another key to acknowledge that, yes, I read the thing you just automatically scrolled me to, highlighted and unfolded.

josch commented 11 years ago

I now also added an "untag unread" in front of every reply command. It now happened to me for a third time that I replied to an email and closed the original email and was then surprised to found the email I just had replied to still to be unread.

pazz commented 11 years ago

i think this last idea should really be the default behaviour, maybe with a config switch to turn it off.

josch commented 11 years ago

Okay. Would you also accept a patch which accept a configuration option that automatically marks an email as read if it is the only unread one in a thread the user opens?

Rationale: if the user opens a thread with only one unread email, then he immediately jumps to that email.

pazz commented 11 years ago

one could couple this with the focus operation in thread buffers.. whenever the buffer focuses a message which is unfolded it removes the unread tag (if not configured to not do this)..

josch commented 11 years ago

another tiny nitpick: the comment for auto_remove_unread in alot.rc.spec says automatically remove 'unread' tag when focussing messages in thread mode. Maybe it should instead say when focussing message bodies.

josch commented 11 years ago

Maybe a better idea: add a thread_message_focus hook. Then within this hook the user is free to do whatever he likes - including marking the focused message unread.

diff --git a/alot/buffers.py b/alot/buffers.py
index 12e0dbc..b28fb49 100644
--- a/alot/buffers.py
+++ b/alot/buffers.py
@@ -428,11 +428,14 @@ class ThreadBuffer(Buffer):

     def set_focus(self, pos):
         logging.debug('setting focus to %s ' % str(pos))
+        msgfocus = settings.get_hook('thread_message_focus')
+        if msgfocus is not None:
+            msgfocus(self, pos, self._tree[pos[0]])
         self.body.set_focus(pos)

     def focus_first(self):
         """set focus to first message of thread"""
-        self.body.set_focus(self._nested_tree.root)
+        self.set_focus(self._nested_tree.root)

     def _sanitize_position(self, pos):
         return self._nested_tree._sanitize_position(pos,
@@ -449,7 +452,7 @@ class ThreadBuffer(Buffer):
         newpos = self._tree.parent_position(mid)
         if newpos is not None:
             newpos = self._sanitize_position((newpos,))
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_first_reply(self):
         """move focus to first reply to currently focussed message"""
@@ -457,7 +460,7 @@ class ThreadBuffer(Buffer):
         newpos = self._tree.first_child_position(mid)
         if newpos is not None:
             newpos = self._sanitize_position((newpos,))
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_last_reply(self):
         """move focus to last reply to currently focussed message"""
@@ -465,7 +468,7 @@ class ThreadBuffer(Buffer):
         newpos = self._tree.last_child_position(mid)
         if newpos is not None:
             newpos = self._sanitize_position((newpos,))
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_next_sibling(self):
         """focus next sibling of currently focussed message in thread tree"""
@@ -473,7 +476,7 @@ class ThreadBuffer(Buffer):
         newpos = self._tree.next_sibling_position(mid)
         if newpos is not None:
             newpos = self._sanitize_position((newpos,))
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_prev_sibling(self):
         """
@@ -488,7 +491,7 @@ class ThreadBuffer(Buffer):
         else:
             newpos = localroot
         if newpos is not None:
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_next(self):
         """focus next message in depth first order"""
@@ -496,7 +499,7 @@ class ThreadBuffer(Buffer):
         newpos = self._tree.next_position(mid)
         if newpos is not None:
             newpos = self._sanitize_position((newpos,))
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def focus_prev(self):
         """focus previous message in depth first order"""
@@ -509,7 +512,7 @@ class ThreadBuffer(Buffer):
         else:
             newpos = localroot
         if newpos is not None:
-            self.body.set_focus(newpos)
+            self.set_focus(newpos)

     def expand(self, msgpos):
         """expand message at given position"""
@@ -553,7 +556,7 @@ class ThreadBuffer(Buffer):
                 MT.expand(MT.root)
                 if first is None:
                     first = (self._tree.position_of_messagetree(MT), MT.root)
-                    self.body.set_focus(first)
+                    self.set_focus(first)
             else:
                 MT.collapse(MT.root)
         self.body.refresh()

This seems to work with this hook:

def thread_message_focus(tbuffer, pos, mt):
    def refresh_widgets():
        mt.refresh()
        tbuffer._auto_unread_dont_touch_mids.add(pos)
        tbuffer.refresh()

    if mt is not None:
        m = mt.get_message()
        m.remove_tags(['unread'], afterwards=refresh_widgets)
        fcmd = commands.globals.FlushCommand(silent=True)
        tbuffer.ui.apply_command(fcmd)

But the code for the hook is probably wrong. I'm not sure what is necessary to set the semaphores correctly.

pazz commented 11 years ago

Quoting josch (2013-05-10 22:25:21)

another tiny nitpick: the comment for auto_remove_unread in alot.rc.spec says automatically remove 'unread' tag when focussing messages in thread mode. Maybe it should instead say when focussing message bodies.

yes, I did not adjust this docstring when I introduced the tree structure..

pazz commented 11 years ago

Quoting josch (2013-05-11 07:32:30)

Maybe a better idea: add a thread_message_focus hook. Then within this hook the user is free to do whatever he likes - including marking the focused message unread.

I think this is overkill. We can always add such a hook later on if at some point someone has a reasonable usecase for it. But as of now I think anybody will want something different from automatically removing unread here. I think the way to go here is to make the auto-remove unread behaviour a little more natural with the minor tweaks you suggested:

Whenever a threadbuffer focusses some message (-summary) via focus_*, check if the message is unfolded, if yes remove unread.

josch commented 11 years ago

I think I like the situation as it is right now (in the master branch) and am now just using a keybinding for "fold; untag unread; move next" to fulfill my needs. You can keep this bug open if you think there remains something to be fixed.