[MERGE] Improve error handling in msgeditor._run_editor.

[Andrew Bennetts]

James Westby wrote:
> On Thu, 2008-12-18 at 10:53 +1100, Andrew Bennetts wrote:
[...]
> > I'm interested in feedback about emitting a warning in this situation.  On
[...]
> 
> I don't think this is a problem.

Thanks!  I appreciate the feedback.

> I do have a couple of small comments about the message though.
[...]
> > +                    trace.warning(
> > +                        "Could not start editor %s: %s (specified by
> > %s)\n"
> > +                        % (candidate, str(e), candidate_source))
> 
> If the editor is specified as "" then this message becomes unclear:
> 
>    Could not start editor :
> 
> Should the value be quoted to indicate that there should be something
> there.

Quoting seems reasonable, so I'll do that.  Note that the message in this
case would actually be:

   Could not start editor : Permission denied (specified by /home/andrew/.bazaar/bazaar.conf)

Which is already good enough, I think.  It certainly points the user in the
right direction.

Hmm, in addition to adding quoting, I might reorder that slightly.  So now
it'll be:

   Could not start editor "" (specified by /home/andrew/.bazaar/bazaar.conf): Permission denied 

> The second is that "$BZR_EDITOR" may not be clear enough for some 
> people, would "$BZR_EDITOR in the environment" or similar be clearer?

I'm inclined to say no.  Most command-line users will recognise the
convention, and for those that don't searching for "BZR_EDITOR" in the doc
or with Google should be enough.  And the message is already pretty wordy.

-Andrew.