Diff broken in 1.0.2

[Shrews]

I believe this change breaks diff functionality in 1.0.2:

332521a

Using OS X and git 2.5.4, the generated command puts a space between the diff -U option and its value. This causes an exception as git insists on there being no space between them.

[Byron]

Thanks for getting down to the root of the problem. This change was introduced to solve a particular (and to me unknown) problem, and I am wondering how to solve this issue without introducing breakage for someone else.

When looking at the code, I couldn't find any usage of the -U option described here.

Can you help me to reproduce the problem ?

[Shrews]

I believe the U option is being passed as part of the kwargs. Actually experiencing this with a tool called gertty. Here is the line with the diff() call where the crash occurs:

https://github.com/openstack/gertty/blob/master/gertty/gitrepo.py#L390-L391

Also, see new comments in: 332521a

[Byron]

What about using the respective long option instead, i.e. --unified ? The line in question would then look like this:

extra_contexts, oldc.diff(
     newc, color='never',create_patch=True, unified=context))

[Shrews]

Yes, the long option works. Buts that's nothing more than a workaround.

[Byron]

Great to hear it works ! Where else are you having problems with that particular behaviour of the git executable when used with gitpython ?
To me it seems the issue is fixed, even without having to change defaults that are likely to break other code elsewhere.

[jeblair]

It might be helpful to know what commit 332521a was attempting to fix. It seems like we have traded something unknown to us being broken for the breakage described in this issue. A quick check of some random diff options suggests that this behavior change will affect not only "-U" in git diff, but also "-C", "-M', and "-B". These are likely to cause GitPython users to run into difficult-to-diagnose problems when they attempt to use these options in their customary form.

With an understanding of what 332521a was fixing, we could evaluate a way forward -- perhaps there is another way to accomplish what was intended there, or perhaps we need to support different behaviors for different git subcommands, or perhaps we need to say that single-char options with arguments are not supported and return a useful error to gitpython users.

[Shrews]

Agree with jeblair here. I find it odd to consider it "fixed" by putting the responsibility on users of the library to modify their once-working code.

[Byron]

Thanks for your feedback !
When looking at #341, it appears that fixing it for one is breaking it for another. Even though a viable solution could be to enforce long options for diff-related commands, I would love to be able to configure this particular flag split_single_char_options on a per-command basis.
That way, the diff implementation could 'fix it by default' and thus prevent the unexpected behaviour experienced here.

Just to unclutter this issue a bit, I would like to move attention to the linked issue, and close this one. In the meanwhile, one could go with the proposed workaround of using long options instead of short ones.