Subject: [sup-talk] [PATCH] Add new :crypto_default configuration option.

From: [email-masked] (Carl Worth)
Date: Mon, 28 Sep 2009 15:57:38 -0700

For example, users that want to sign all outgoing messages by default
can set:

	:crypto_default: :sign

in ~/.sup/config.yaml. Configuring an invalid value will cause a list
of the valid values to be logged at the "error" level.
---
 lib/sup/horizontal-selector.rb     |    8 +++++++-
 lib/sup/modes/edit-message-mode.rb |    7 ++++++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/lib/sup/horizontal-selector.rb b/lib/sup/horizontal-selector.rb
index aef16d4..13c63ed 100644
--- a/lib/sup/horizontal-selector.rb
+++ b/lib/sup/horizontal-selector.rb
@@ -12,7 +12,13 @@ class HorizontalSelector
     @selection = 0
   end
 
-  def set_to val; @selection = @vals.index(val) end
+  def set_to val
+    if @vals.index(val)
+      @selection = @vals.index(val)
+    else
+      error "Invalid option ", val.inspect, " (valid options: ", @vals.inspect, ")"
+    end
+  end
 
   def val; @vals[@selection] end
 
diff --git a/lib/sup/modes/edit-message-mode.rb b/lib/sup/modes/edit-message-mode.rb
index 8da316f..3449503 100644
--- a/lib/sup/modes/edit-message-mode.rb
+++ b/lib/sup/modes/edit-message-mode.rb
@@ -89,7 +89,12 @@ EOS
       if CryptoManager.have_crypto?
         HorizontalSelector.new "Crypto:", [:none] + CryptoManager::OUTGOING_MESSAGE_OPERATIONS.keys, ["None"] + CryptoManager::OUTGOING_MESSAGE_OPERATIONS.values
       end
-    add_selector @crypto_selector if @crypto_selector
+    if @crypto_selector
+      if !$config[:crypto_default].nil?
+        @crypto_selector.set_to $config[:crypto_default]
+      end
+      add_selector @crypto_selector
+    end
     
     HookManager.run "before-edit", :header => @header, :body => @body
 
-- 
1.6.4.3
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20090928/cc5a4c98/attachment.bin>

---

From: [email-masked] (William Morgan)
Date: Thu, 01 Oct 2009 10:07:20 -0700

I like the idea, but how about a hook instead? I think the reply-mode
hook is exactly equivalent to this. (Which maybe I will one day rename
to default-reply-mode.)

I have a strong aversion to adding configuration options.
-- 
William <wmorgan-sup at masanjin.net>

---

From: [email-masked] (Carl Worth)
Date: Thu, 01 Oct 2009 10:31:00 -0700

> I have a strong aversion to adding configuration options.

I'm intrigued.

What makes a hook preferable over a configuration option?

I've been getting concerned watching the number of hooks in sup grow
as each creates a maintenance burden. Either:

1. All hooks are supported forever with consistent
   arguments/semantics, (which may make it more difficult to make
   changes in sup than it would be otherwise)

OR:

2. Hooks are not supported forever, in which case users may find that
   things just start working when upgrading.

Neither of those seem options look nice to me, and both seem easy to
avoid with configuration options.

If the plan is to go with (1) I'm concerned that I don't see sup
shipping documentation for the current possible hooks. (This applies
to configuration options too though. I think the maintainer should
reject patches that add either without also adding documentation to
the standard list.[*])

[*] Assuming the pre-condition of such documentation existing of
course.

On the other hand, I also dislike configuration options (and hooks,
equally), to the extent that they might be used as an excuse to avoid
putting the most sane and useful default functionality into sup
itself. Obviously, this can be complicated by some people not agreeing
on what the most sane and useful behavior is.

-Carl
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 190 bytes
Desc: not available
URL: <http://rubyforge.org/pipermail/sup-talk/attachments/20091001/55fc7ab2/attachment.bin>