Subject: [sup-talk] Sup annoyances

From: [email-masked] (Edward Z. Yang)
Date: Sat, 06 Jun 2009 02:54:01 -0400

Having just submitted my first patch, I feel entitled to gripe and
moan about certain aspects of Sup. The hope is that I'll get some
pointers on what code to look at to fix these gripes. These gripes
are independent of the hanging/other behavior in my other monster
thread. (I'd say the asynchronous polling hang is my other biggest
gripe, but that's not an issue for here).

1. Threads should be lazy-loaded (possibly in the background), with
   visible messages first.  If a conversation got a new message, Sup
   should only take its time to load that message, and not the frickin'
   24 other messages that were also incidentally there.  I suspect
   this will require some pretty major refactoring.  As a stop-gap,
   I should have the option of cancelling when I open a long thread
   (right now, I think it causes sup to crash).

2. Auto-completion sucks and should be improved.  I should be able
   to press "c", type two letters, and then mash tab. If auto-completion
   actually works, then I'll blame it on dismal contacts.txt support
   (if I send mail to someone, there should be a configuration twiddle
   that saves it to contacts.txt. And then remembers it the next
   time I want to compose mail to them).

3. Polling and loading of threads should started, asynchronously, at
   the same time.  Loading of threads should not hose my CPU.  Loading
   of threads should do smart things, instead of doing an O(n) time warp.
   It takes forever for a mail from two months ago that is still in my
   inbox, because it still hasn't been addressed, to show up.  This could
   probably be fixed with a cache file.  This isn't as big of a deal
   if asynchronous polling hang and crashing is fixed.

4. Sup should prompt me whether or not I want to Reply All or Reply on
   appropriate cases, like pine.

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Sat, 06 Jun 2009 02:56:11 -0400

>    appropriate cases, like pine.

To be clear, this should be a configuration twiddle.

I think I also have just remembered one more thing:

5. Killing a thread should kill it *now*, not "when Sup next restarts".

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Sat, 06 Jun 2009 14:17:29 -0400

Excerpts from Marc Hartstein's message of Sat Jun 06 13:16:39 -0400 2009:
> sup gives you a menu to select whether to reply to Sender, Recipient,
> List, or All.  It's right up near the top of the reply-mode screen.  Use
> regular vertical movement keys to select the Reply to: line, then
> horizontal movement to select an option.

Yep, I know about this.

> You can improve the default selection using the reply-to hook.  See `sup
> --list-hooks` for documentation.

I didn't know about that.

> I found it a little weird getting used to (coming from mutt, where I
> actually used a different reply key for each mode), but the default
> behavior is generally correct, and selecting from the menu isn't too
> bad.

The trouble is I always forget to check the To: header on my mails, and
there is a ~large class of mailing list emails that don't actually do
the right thing by default.  Improving the heuristic may be an alternate
possibility.

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Mon, 08 Jun 2009 11:00:23 -0700

Hi Edward,

I think these are by and large legitimate gripes and I'd be delighted to
have someone motivated to fix them!

Reformatted excerpts from Edward Z. Yang's message of 2009-06-05:
>    visible messages first.

FWIW, this is particularly egregious in a direct IMAP connection, and
those of us who have switched to Maildir or who use mbox aren't
punished. But it would be very cool to have this.

> If a conversation got a new message, Sup should only take its time to
> load that message, and not the frickin' 24 other messages that were
> also incidentally there.  I suspect this will require some pretty
> major refactoring.

It might not be too bad. Turn off the call to m.load_from_source! on
line 106 of thread-index-mode.rb, add a Queue to ThreadViewMode
containing messages to call #load_from_source! on, don't call m.chunks
in ThreadViewMode#regen_text unless the message has been loaded, start a
thread that pops messages from the queue, calls #load_from_source!  on
them, and then calls regen_text when an open message has been loaded...
and you're 90% of the way there.

> actually works, then I'll blame it on dismal contacts.txt support

The auto-completion is awesome. Adding a recipient to the contacts list is
a good idea.

>    the same time.

> Loading of threads should not hose my CPU.

Agreed. Please fix.

> Loading of threads should do smart things, instead of doing an O(n)
> time warp.

It's definitely worse than O(n). Loading threads could be sped up
dramatically by storing the thread structure somewhere (either cached or
just for every thread), since Sup does a lot of extra work rethreading
everything every time you start it up. FWIW I'm doing this the right way
in Sup 2.0.

>    appropriate cases, like pine.

There have been a couple similar requests like this recently, though
it's not clear if everyone has known about the ability to change the
reply mode and about the reply-to hook.  Since hitting 'r' defaults to
replying to the sender alone (except for mailing list messages), I'd be
ok with adding a Mutt-style reply-to-all key that starts up reply-mode
with the 'reply to all' mode active.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 19:14:32 -0400

> and you're 90% of the way there.

A few questions:

* How do I tell if a message is shown on the screen or not?

* The implementation you describe still downloads the messages
  sequentially, which are usually not the messages we want to download
  first. I don't mind sup blocking on downloading a message, as long
  as it's one or two, which means I don't strictly need threads: I
  just need some API for, when a new message is opened, downloading it
  and then rendering it.  What do you think of this?

> The auto-completion is awesome. Adding a recipient to the contacts list is

Does contacts.rb apply to people.txt or contacts.txt?

> everything every time you start it up. FWIW I'm doing this the right way

Can you backport it easily to Sup 1.0?

> with the 'reply to all' mode active.

The explicit prompt makes me think about who I want to send the message
to.  I frequently forget to move the little knob with Sup's behavior.

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Mon, 08 Jun 2009 16:46:39 -0700

> * How do I tell if a message is shown on the screen or not?

If m is the message, @layout[m].state will be :open, :detailed or
:closed. If it's not :closed, it is visible.

> first.

Only if you add them to the Queue sequentially. You could instead add
the ones with :unread or :starred labels first. And ideally if you
expand a collapsed unloaded message, that should be pushed to the head
of the Queue.

> I don't mind sup blocking on downloading a message, as long

So you would only preload the new/starred ones, and load the others only
when they're expanded? I think that simply removing that call in
thread-index-mode.rb would accomplish this.

I'd rather have the threaded version, since loading a thread of 100 new
messages sucks, but either way would be an improvement.

> > The auto-completion is awesome. Adding a recipient to the contacts
> > list is a good idea.
> 

contacts.txt. people.txt is deprecated. In fact I'm surprised you have
one.

> > dramatically by storing the thread structure somewhere (either
> > cached or just for every thread), since Sup does a lot of extra work
> > rethreading everything every time you start it up. FWIW I'm doing
> > this the right way in Sup 2.0.
> 

No, it's completely different. It would be easier to rewrite from
scratch for the existing Sup.

> The explicit prompt makes me think about who I want to send the
> message to. I frequently forget to move the little knob with Sup's
> behavior.

When do you want it to prompt you? Is this a yes/no prompt?
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 22:13:17 -0400

Thanks for the answers to the technical questions.

> messages sucks, but either way would be an improvement.

I am somewhat skeptical of your described change, but I will try it
out and report back.  The threaded version would certainly be a nice
thing.

> contacts.txt. people.txt is deprecated. In fact I'm surprised you have

Um... my contacts.txt is empty, and people.txt has lots of entries in it.

> scratch for the existing Sup.

Savvy.

> When do you want it to prompt you? Is this a yes/no prompt?

Yes. (to yes/no).

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 22:23:44 -0400

> out and report back.  The threaded version would certainly be a nice

It works, although it acts a bit strangely when there are still
threads loading into memory.

>From b2518f113118e489e22a1c2085b33a9c18721f97 Mon Sep 17 00:00:00 2001
From: Edward Z. Yang <edwardzyang at thewritingpot.com>
Date: Mon, 8 Jun 2009 22:21:01 -0400
Subject: [PATCH] Remove message pre-loading; optimizes for the common case.
 Signed-off-by: Edward Z. Yang <edwardzyang at thewritingpot.com>

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

diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
index 9b44ee3..29de39a 100644
--- a/lib/sup/modes/thread-index-mode.rb
+++ b/lib/sup/modes/thread-index-mode.rb
@@ -103,7 +103,6 @@ EOS
         t.each_with_index do |(m, *o), i|
           next unless m
           BufferManager.say "#{message} (#{i}/#{num})", sid if t.size > 1
-          m.load_from_source! 
         end
       end
       mode = ThreadViewMode.new t, @hidden_labels, self
-- 
1.6.0.4

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 22:35:04 -0400

> threads loading into memory.

I think I've been able to pin down this behavior more precisely,
and it's not just this patches fault: displaying messages seems to
block on having all of the threads present.  Does this ring a bell?

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 22:38:08 -0400

> > Loading of threads should not hose my CPU.

It appears that polling blocks on loading threads, so it doesn't help
to fix that until we fix the thread loading code.

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Mon, 08 Jun 2009 23:00:55 -0400

> everything every time you start it up. FWIW I'm doing this the right way

I'm thinking about how to do this, and one of the first I'm blockers
I'm running against is that it's not quite obvious what's invoking
the thread building code thread.rb.

As for caching the thread, I'm thinking of generating a unique
ID for each thread and sticking that on every message.  Whether or
not this will actually help depends a lot on what the answer to the
above question is.

Cheers,
Edward

---

From: [email-masked] (William Morgan)
Date: Tue, 09 Jun 2009 10:32:53 -0700

> Um... my contacts.txt is empty, and people.txt has lots of entries in it.

Does your people.txt have a really old timestamp? That file doesn't get
touched by any version of Sup later less than three months old.
Do you add people to your contact list with 'i'? If not, contacts.txt
will be empty.

> > When do you want it to prompt you? Is this a yes/no prompt?

So... once you write the email and you press 'y' it prompts you whether
you want to send it? Or when you press 'r' it prompts you whether you
really want to reply?
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Tue, 09 Jun 2009 10:40:38 -0700

> block on having all of the threads present.  Does this ring a bell?

Not entirely sure what you mean by "having all the threads present", but
opening a thread-view-mode buffer doesn't block on anything, at least
with that patch applied. Without the patch, it blocks on loading the
message body for every message in that particular thread from the
server.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Tue, 09 Jun 2009 10:45:45 -0700

Reformatted excerpts from Edward Z. Yang's message of 2009-06-08:
> > > 3. Polling and loading of threads should started, asynchronously,
> > > at the same time.
> > 

My "agreed, please fix" was in response to "loading of threads should
not hose my CPU".

Polling currently blocks on loading threads, but that's an explicit
block. I forget why I did that now. You can unblock it and see if it
makes life better.  (bin/sup circa line 205.)
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (William Morgan)
Date: Tue, 09 Jun 2009 10:48:59 -0700

> Subject: [PATCH] Remove message pre-loading; optimizes for the common case.

The problem with this patch as is is that loading a large thread
composed of entirely new messages still requires loading all the message
bodies from the server, which can take a couple seconds, but now there's
no indication of what's happening. So either the fancy background
loading needs to happen, or somehow a message needs to be displayed when
the bodies are being loaded.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Edward Z. Yang)
Date: Wed, 10 Jun 2009 23:18:46 -0400

> message body for every message in that particular thread from the

Yeah. I mean what I mean: it *shouldn't* block on anything, but as
far as I can tell from manual testing, thread-view-mode doesn't open
up until I'm done threading the index.  Maybe that's why.

Cheers,
Edward

---

From: [email-masked] (Edward Z. Yang)
Date: Wed, 10 Jun 2009 23:19:32 -0400

> the bodies are being loaded.

Understood. There's also another odd bug where the headers for the open
messages "blow up" (get split into three lines each) which gets fixed
when you collapse and the expand the headers again.

Cheers,
Edward