Subject: [sup-talk] [PATCH] don't try to open the next thread if there isn't one available

From: [email-masked] (Jeff Balogh)
Date: Wed, 30 Jan 2008 03:21:18 -0500

When you're on the last thread of a ThreadSet and press ',<key>',
nothing happens, but you can't '.<key>' since @dying == true.  This
patch changes :next to :kill if there is no next thread.
---
I usually traverse my inbox with ',a', so it annoys me when sup gets stuck on
the last thread.  This patch implements what I expect to happen: the thread is
killed and the previous buffer pops up.

 lib/sup/modes/thread-index-mode.rb |    4 ++++
 lib/sup/modes/thread-view-mode.rb  |    2 ++
 2 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index 7686098..d6c1b21 100644
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -121,6 +121,10 @@ EOS
 
     select t, b
   end
+
+  def has_thread_after thread
+      @lines[thread] < @threads.length - 1
+  end
   
   def handle_single_message_labeled_update sender, m
     ## no need to do anything different here; we don't differentiate 
diff --git a/lib/sup/modes/thread-view-mode.rb b/lib/sup/modes/thread-view-mode.rb
index 961fc5d..e79e410 100644
--- a/lib/sup/modes/thread-view-mode.rb
+++ b/lib/sup/modes/thread-view-mode.rb
@@ -416,6 +416,8 @@ EOS
     return if @dying
     @dying = true
 
+    op = :kill unless @index_mode.has_thread_after(@thread)
+
     case op
     when :next
       @index_mode.launch_next_thread_after(@thread) do
-- 
1.5.3.7

---

From: [email-masked] (John Bent)
Date: Wed, 30 Jan 2008 08:43:33 -0700

> killed and the previous buffer pops up.
Perhaps I should just change my habits but I like to traverse my inbox in
chronological order (i.e. bottom up instead of top down).  If anyone could 
make a patch that does ,a or ,n but moves it up instead of down, I'd very
much appreciate it.

By the way, I finally figured out what n and p are for in thread-view mode.
They are really very nice.  The auto-topping and auto-margin positioning are
very slick.

John

---

From: [email-masked] (Jeff Balogh)
Date: Wed, 30 Jan 2008 18:05:40 -0500

> killed and the previous buffer pops up.

Ok, this patch causes a no block given exception with ',n' on the last thread.
As you might expect, no block is given with the do nothing command.

Here's where it's happening:
  def dispatch op
    return if @dying
    @dying = true

    op = :kill unless @index_mode.has_thread_after(@thread)

    case op
    when :next
      @index_mode.launch_next_thread_after(@thread) do
        @thread.save Index if block_given? && yield
        BufferManager.kill_buffer_safely buffer
      end
    when :kill
      @thread.save Index if yield
      BufferManager.kill_buffer_safely buffer
    else
      raise ArgumentError, "unknown thread dispatch operation #{op.inspect}"
    end
  end

The :next case guards against this exception with 'if block_given? && yield'.  If
I put that in the :kill case, those two lines will be exact duplicates of the
lines above.

Is there a clean way to handle this?  I'm learning Ruby as I go along, so please
forgive me if the answer is obvious.

---

From: [email-masked] (William Morgan)
Date: Sat, 02 Feb 2008 20:31:00 -0800

> patch changes :next to :kill if there is no next thread.

I made a parallel change to do this:
e47f9dabca0349a1fff046232961b38d900d4445. It's all withing
ThreadIndexMode#launch_next_thread_after, which I think is a little
cleaner.

As I mentioned in the previous email, I also just changed these
operations so that they don't automatically save state to disk, so as to
be consistent with the rest of Sup.

-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Sat, 02 Feb 2008 20:35:33 -0800

Reformatted excerpts from its.jeff.balogh's message of 2008-01-30:
> The :next case guards against this exception with 'if block_given? &&
> yield'.  If I put that in the :kill case, those two lines will be
> exact duplicates of the lines above.
> 
> Is there a clean way to handle this?  I'm learning Ruby as I go along,
> so please forgive me if the answer is obvious.

To answer your question, depends what you mean by clean. We can capture
the shared lines in a closure with lambda { ... }, or we can move them
to a function and call the function, but that's basically the extent of
the control flow power we have here. (Well, unless we want to start
using continuations!)

In the latest version of the code, the shared lines are now exactly the
same, and are simply duplicated. Maybe once a :prev branch is added a
refactor will be necessary.

---

From: [email-masked] (William Morgan)
Date: Tue, 05 Feb 2008 09:27:26 -0800

> make a patch that does ,a or ,n but moves it up instead of down, I'd very

I do this too, often. Try ']' in next.

I really need a better keybinding. Wonder if I just should bite the
bullet and do < and > for previous and next instead of , and ].

---

From: [email-masked] (John Bent)
Date: Wed, 06 Feb 2008 09:21:37 -0700

> I do this too, often. Try ']' in next.
That's great!  Thanks.

> bullet and do < and > for previous and next instead of , and ].
That'd be nice too (and probably more intuitive).  Or maybe it's really
hard but user-defined macros or shortcuts.  So people could map
< to ,n and > to ]n.  Or did I get the order wrong?  Also, if we had
macros, I'd map something to |muttprint.