[MERGE] Improve error handling in msgeditor._run_editor.
James Westby
jw+debian at jameswestby.net
Thu Dec 18 17:13:21 GMT 2008
On Thu, 2008-12-18 at 10:53 +1100, Andrew Bennetts wrote:
> A user on IRC just reported trouble using “bzr commit” without -m. It
> turned out that somehow they had this line in their bazaar.conf:
>
> editor = ""
>
> There are two issues here. First is that executing "" causes EACCES, and
> _run_editor wasn't catching that. The second is that the error reporting
> was very unhelpful in this case; it didn't report which command bzr was
> trying to run, or why.
> I'm interested in feedback about emitting a warning in this situation. On
> one hand, it would have made this problem easier to diagnose — the user on
> IRC today probably wouldn't have needed to come to #bzr for help if we
> emitted it. But maybe it's too chatty to complain if a user's environment
> is slightly broken? I think probably this change is fine (it's usually not
> hard to set $EDITOR correctly), but I know extra output can irritate people,
> so let me know.
I don't think this is a problem.
I do have a couple of small comments about the message though.
> + if e.errno in (errno.ENOENT, errno.EACCES):
> + if candidate_source is not None:
> + # We tried this editor because some user
> configuration (an
> + # environment variable or config file) said to
> try it. Let
> + # the user know their configuration is broken.
> + 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.
The second is that "$BZR_EDITOR" may not be clear enough for some
people, would "$BZR_EDITOR in the environment" or similar be clearer?
Thanks,
James
More information about the bazaar
mailing list