[MERGE][#120050] Give and example of --fixes in the help for the option.

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

James Westby wrote:
| On Tue, 2008-07-29 at 21:00 -0500, John Arbash Meinel wrote:
|> I think either "bzr help commit" or the error message should tell you
|> about "bzr help bugs". Rather than just "tracker:id".
|>
|
| Commit has a "See also: bugs", do you think it would be
| better to change "(e.g. --fixes=lp:2345)" to '(see "bzr help
| bugs")'?

Yeah.

|
|>  class MalformedBugIdentifier(BzrError):
|>
|> -    _fmt = "Did not understand bug identifier %(bug_id)s: %(reason)s"
|> +    _fmt = ('Did not understand bug identifier %(bug_id)s:
|> %(reason)s. '
|> +            'See "bzr help bugs" for more information on this
|> feature')
|>
|
| So you get told if you do "bzr commit --fixes 21345", but you
| don't get told if you do "bzr commit --fixes bogus:1324" or
| "bzr commit --fixes lp:word". I thought these were less important,
| but do you think they should have the pointer anyway?
|
| Thanks,
|
| James
|

I think you have your exceptions wrong. With your patch:

$ jbzr commit --fixes foo
bzr: ERROR: Invalid bug foo. Must be in the form of 'tracker:id'. Commit
refused.

Nothing about help bugs

$ jbzr commit --fixes 1234
bzr: ERROR: Invalid bug 1234. Must be in the form of 'tracker:id'.
Commit refused.

Nothing about help bugs

$ jbzr commit --fixes lp:word
bzr: ERROR: Invalid bug identifier for lp:word. Commit refused.

Nothing about help bugs (this is a bit harder, because depending on the
tracker, it might not use numbers, though I haven't run into one yet
that worked that way.)

$ jbzr commit --fixes not-a-prefix:word
bzr: ERROR: Unrecognized bug not-a-prefix:word. Commit refused.

Nothing about help bugs

If you look at the code, it is doing:

~            try:
~                bug_url = bugtracker.get_bug_url(tag, branch, bug_id)
~            except errors.UnknownBugTrackerAbbreviation:
~                raise errors.BzrCommandError(
~                    'Unrecognized bug %s. Commit refused.' % fixed_bug)
~            except errors.MalformedBugIdentifier:
~                raise errors.BzrCommandError(
~                    "Invalid bug identifier for %s. Commit refused."
~                    % fixed_bug)

So it is catching your exception and re-wrapping them with a less
helpful one.

My #1 concern is this exception:

~           if len(tokens) != 2:
~                raise errors.BzrCommandError(
~                    "Invalid bug %s. Must be in the form of 'tracker:id'. "
~                    "Commit refused." % fixed_bug)

Which is what you get from "bzr commit --fixes foo" or "bzr commit
- --fixes 12345".

Because that is what users that don't know what is happening are going
to hit first.

The others are also important, but less so.

Also, I suggest you actually try these out on the command line / add
blackbox tests for them. Because it *looks* like patching
MalformedBugIdentifer fixes the problem, but it doesn't, because that
format string is ignored.

(The nice thing about supplying an invalid --fixes is that it will abort
the commit anyway, so you can keep re-trying with different ones to see
the command line behavior, without actually committing anything.)

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkiQiTQACgkQJdeBCYSNAAO8nACgnEY8sP/e8BTvfKICmIYQAEDU
+4UAnj0XqCPID5V7xb+B0XV1JzfXZHJW
=aiaD
-----END PGP SIGNATURE-----