Subject: [sup-devel] [PATCHES] Add an account selector in edit-mode

From: Damien Leone <[email-masked]>
Date: Sun, 13 Mar 2011 17:13:05 +0100

Sup guys,

Please see commit messages for detailed informations.

-- 
Damien Leone <[email-masked]>

Web: http://dleone.fensalir.fr/
GPG: 0x82EB4DDF
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: Hamish Downer <[email-masked]>
Date: Mon, 20 Jun 2011 22:38:21 +0100

Excerpts from Sascha Silbe's message of Mon Jun 20 14:47:57 +0100 2011:
> This breaks the "Reply to" selector for me. Any attempt to change the
> value results in an exception:
> 
> So account_selector.selection is nil. The only way for that to happen is
> by calling account_selector.set_to() with a value that's not in
> account_selector.vals (IMO HorizontalSelector should throw an exception
> in that case, BTW). Any idea where and why that would have happened?

I'm afraid I couldn't reproduce your exception - do you have several
accounts? Multiple email addresses per account?

Anyway I've pushed an update to the account_selector branch (commit
5bb55890) that you could try out. The basic change is that in the
original patch, there was:

    @account_selector =
      HorizontalSelector.new "Account:", AccountManager.user_accounts + [nil], user_emails_copy + ["Customized"]

Note the [nil] added to the end of the first array. I've changed that to
[:user] (in line with the replyto selector) and made the other changes
in the file to stay in line with that. As I say I can't reproduce your
problem, so I'm not 100% sure I've solved it, so let me know how you get
on. If it fixes it for you I'll put the patch onto next aswell.

Hamish Downer

---

From: Damien Leone <[email-masked]>
Date: Mon, 20 Jun 2011 10:08:39 +0200

Hello,

Sorry I have been quite busy finishing my degree last weeks. I planned
to review and answer but it got delayed and delayed.

Thanks for pushing and fixing the issue with alternate addresses. I
will fetch it and let you know.

Excerpts from Hamish's message of dim. juin 19 23:48:40 +0200 2011:
> Excerpts from Hamish's message of Mon May 30 18:23:10 +0100 2011:
> > Excerpts from Damien Leone's message of Sun Mar 13 16:13:05 +0000 2011:
> > > 0002-edit-message-mode-Add-an-optional-account-selector.patch
> > 
> > I imagine though that you would want to handle all the possible email
> > addresses, so I've written an extra patch to do that, and pushed your
> > patch and mine onto the (new) account_selector branch on gitorious.
> > Please have a look and see if what I've done still works for you, and is
> > what you meant.
> > 
> > I also wondered whether to change the behaviour so that the account
> > selector will only appear if there is more than one possible email
> > address to choose from, but I thought it would be handy to be able to
> > reset the From: header to your usual From: using the selector, rather
> > than having to hand edit whatever was in the To: header that you are
> > replying to. What do you think?
> > 
> > I'll wait for you to review my changes before I merge this into next.
> > (My changes are in commit bf56b543cc664c40c3695e287ee8e7e924f2d0d1)
> 
> I've not heard anything for a bit, so I've pushed this into next for
> wider testing, with the guard so that the account selector will only
> appear if you have more than one email address. Shout out if it causes
> any problems.
> 
> Hamish

GPG: 0x82EB4DDF

---

From: Hamish <[email-masked]>
Date: Sun, 19 Jun 2011 22:48:40 +0100

Hamish

---

From: Sascha Silbe <[email-masked]>
Date: Sun, 13 Mar 2011 21:21:06 +0100

> Attachment: 0001-reply-mode-improve-the-way-headers-are-handled.patch

This one would possibly break my workflow. My From address is chosen in
the before-edit hook based on the set of recipients. Since you only
call before-edit once with headers_full as headers, whatever From
address gets chosen for the initial reply mode will "stick".

It also seems that something I wanted to do for some time was already
possible with the old code, but isn't anymore with yours: Changing the
contents of the To and CC fields for the selector. Even arbitrary headers
(Mail-Followup-To in my case) could be keyed on the selector. I now
understand why before-edit was invoked multiple times. :)

And the part where I need to avoid modifying the body on subsequent
invocations looks like a bug in the original code: @bodies[k] should
contain a _copy_ of body (currently all @bodies[k] contain a reference
to the same instance, rendering the Hash useless).

BTW, do you know about Hash.merge! ? Some parts of your code could have
been simplified by using merge! instead of merge.

Sascha

-- 
http://sascha.silbe.org/
http://www.infra-silbe.de/
_______________________________________________
Sup-devel mailing list
[email-masked]
http://rubyforge.org/mailman/listinfo/sup-devel

---

From: Hamish <[email-masked]>
Date: Mon, 30 May 2011 18:23:10 +0100

> 0002-edit-message-mode-Add-an-optional-account-selector.patch

I've finally got around to reviewing some of these. The patch above
seems to confuse possible email addresses, and user accounts. These do
not line up as it is possible for each account to have multiple email
addresses associated with it (using the alternates value in an account).

I have only one account, and have alternates, so the account selector
appears, but doesn't work due to the confusion. I've had a look through,
but not found an easy way to fix that behaviour, so I'll leave it up to
you as to how you want to do it.

Hamish Downer

---

From: Sascha Silbe <[email-masked]>
Date: Mon, 20 Jun 2011 15:47:57 +0200

--- TypeError from thread: main
no implicit conversion from nil to integer
./lib/sup/horizontal-selector.rb:18:in `[]'
./lib/sup/horizontal-selector.rb:18:in `val'
./lib/sup/modes/edit-message-mode.rb:374:in `update'
./lib/sup/modes/edit-message-mode.rb:351:in `move_cursor_left'
./lib/sup/modes/reply-mode.rb:174:in `move_cursor_left'
./lib/sup/mode.rb:59:in `send'
./lib/sup/mode.rb:59:in `handle_input'
./lib/sup/buffer.rb:278:in `handle_input'
bin/sup:271

Reverting the account selector patches makes it work again.

This is the code in edit-message-mode.rb:

372  def update
373    if @account_selector
374      if @account_selector.val.nil?
375        @header["From"] = @account_user
376      else
377        @header["From"] = AccountManager.full_address_for @account_selector.val
378      end
379    end

And horizontal-selector.rb:

18   def val; @vals[@selection] end

Sascha