Code review comment for lp:~jelmer/bzr/diff-git

On Mon, 2010-12-20 at 07:37 +0000, Martin Pool wrote:
> On 20 December 2010 17:16, Andrew Bennetts
> <email address hidden> wrote:
> > I'm +0 on the UI change... I think it's probably ok, but I'm a big user of non-default diff formats so I'd feel more comfortable if Martin's impeccable UI taste says so too. Martin?
> >
> > The diff itself is reasonable, although you should add release-notes and whats-new entries.
>
> Generally speaking I'm not a big fan of format_switches, because
>
> - it makes the help a bit less readable (though that could in
> principle be fixed)
> - it looks like there are more options than there actually are
> - related, values that are reasonable option arguments cause trouble
> with sphinx etc when presented as actual options, eg things that
> contain a dot
> - if we get an unknown option (because eg we don't have a plugin
> installed) it is a bit harder to explain why (though this is only
> theoretical because we don't currently explain why in this case)
>
> I wonder here if we could instead support say -Fgit? Would that both
> work, and be reasonably concise?
Hmm, that's true. I guess there's a bit of hypocrisy here on my part as
I've complained about format switches myself in the past, in particular
in upgrade.

A short format option would be nice.

Another reason for having value switches was discoverability. I wonder
if we could perhaps list the possible values in the help string for a
RegistryOption if value_switches is not set? At the moment "bzr diff
--format=git" is not very discoverable. I'm thinking along the lines of:

...
  --format=ARG Diff format to use. (unified, git) [unified]
...

Cheers,

Jelmer