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
Excerpts from Edward Z. Yang''s message of Sat Jun 06 02:54:01 -0400 2009:> 4. Sup should prompt me whether or not I want to Reply All or Reply on > 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
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
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:> 1. Threads should be lazy-loaded (possibly in the background), with > 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.> 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 supportThe auto-completion is awesome. Adding a recipient to the contacts list is a good idea.> 3. Polling and loading of threads should started, asynchronously, at > 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.> 4. Sup should prompt me whether or not I want to Reply All or Reply on > 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>
Excerpts from William Morgan''s message of Mon Jun 08 14:00:23 -0400 2009:> 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.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 > a good idea.Does contacts.rb apply to people.txt or contacts.txt?> 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.Can you backport it easily to Sup 1.0?> 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.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
Reformatted excerpts from Edward Z. Yang''s message of 2009-06-08:> * 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.> * The implementation you describe still downloads the messages > sequentially, which are usually not the messages we want to download > 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 > 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?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. > > Does contacts.rb apply to people.txt or contacts.txt?contacts.txt. people.txt is deprecated. In fact I''m surprised you have one.> > 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. > > Can you backport it easily to Sup 1.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>
Thanks for the answers to the technical questions. Excerpts from William Morgan''s message of Mon Jun 08 19:46:39 -0400 2009:> 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.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 > one.Um... my contacts.txt is empty, and people.txt has lots of entries in it.> No, it''s completely different. It would be easier to rewrite from > scratch for the existing Sup.Savvy.> > 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?Yes. (to yes/no). Cheers, Edward
Excerpts from Edward Z. Yang''s message of Mon Jun 08 22:13:17 -0400 2009:> 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.It works, although it acts a bit strangely when there are still threads loading into memory.>From b2518f113118e489e22a1c2085b33a9c18721f97 Mon Sep 17 00:00:00 2001From: 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
Excerpts from Edward Z. Yang''s message of Mon Jun 08 22:23:44 -0400 2009:> It works, although it acts a bit strangely when there are still > 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
Excerpts from William Morgan''s message of Mon Jun 08 14:00:23 -0400 2009:> > 3. Polling and loading of threads should started, asynchronously, at > > the same time. > > > Loading of threads should not hose my CPU. > > Agreed. Please fix.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
Excerpts from William Morgan''s message of Mon Jun 08 14:00:23 -0400 2009:> 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.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
Reformatted excerpts from Edward Z. Yang''s message of 2009-06-08:> Excerpts from William Morgan''s message of Mon Jun 08 19:46:39 -0400 2009: > > contacts.txt. people.txt is deprecated. In fact I''m surprised you have > > one. > > 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? > > Yes. (to yes/no).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>
Reformatted excerpts from Edward Z. Yang''s message of 2009-06-08:> 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?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>
Reformatted excerpts from Edward Z. Yang''s message of 2009-06-08:> Excerpts from William Morgan''s message of Mon Jun 08 14:00:23 -0400 2009: > > > 3. Polling and loading of threads should started, asynchronously, > > > at the same time. > > > > > Loading of threads should not hose my CPU. > > > > Agreed. Please fix. > > It appears that polling blocks on loading threads, so it doesn''t help > to fix that until we fix the thread loading code.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>
Reformatted excerpts from Edward Z. Yang''s message of 2009-06-08:> 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.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>
Excerpts from William Morgan''s message of Tue Jun 09 13:40:38 -0400 2009:> 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.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
Excerpts from William Morgan''s message of Tue Jun 09 13:48:59 -0400 2009:> 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.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