Re: git-format-patch possible regressions

From: Junio C Hamano <junkio@cox.net>
Date: May 25, 2006, 19:56 UTC
Message-ID: <7vhd3dubd9.fsf@assigned-by-dhcp.cox.net>
In-Reply-To: <e5bfff550605251223g2cf8cfb9vfa18d016b369188d@mail.gmail.com>

"Marco Costalba" <mcostalba@gmail.com> writes:

Show 12 quoted lines

> A couple of possible regressions:
>
> 1) Unreconized --signoff option
>
> $ git --version
> git version 1.3.3.ged90
> $ git-format-patch -s HEAD^..HEAD
> 0001-cat-file-document-p-option.txt
> $ git-format-patch --signoff HEAD^..HEAD
> fatal: unrecognized argument: --signoff
>...
> Both regressions brake qgit. The first one is easy to fix (--signoff  --> -s)

I do not think -s does what you want. It means "do not generate diff" to the diff family, but format-patch overrides it and forces generating patch+stat output, so you do not see what it is doing.

Also I do not think we would want to have --sign to format-patch anyway; it encourages a wrong workflow. Please see this and other messages in the thread:

	http://article.gmane.org/gmane.comp.version-control.git/20389

On a slightly related topic, I sent a message to Pasky about this -s stuff. It means something slightly different in diff-files (instead of asking for no output, it behaves as a no-op there), and we can remove that compatibility wart once Cogito stops using "diff-files -s" when it wants to do "diff-files" in cg-merge (and I suspect that diff-files is unnecessary).

> 2) Unhandled ranges list
>
> The second one is not so easy.

This is a real regression; I was hoping Porcelain writers were paying attention of what are coming, but obviously the description in "What's in git.git" messages and discussion on the list were not detailed enough. My apologies.

Having said that, I think what Linus says about equilvalence between "a..b" and "^a b" makes whole lot of sense. However, I could argue both ways. Linus's interpretation of "a..b c..d" is "^a ^c b d", but format-patch's interpretation has always been "do '^a b' and then '^c d'".

The former is more generic; you could say "not in A nor B but in C pretty easily -- list of ranges cannot express something like that. On the other hand, at least in the context of usual format-patch, the convenience of being able to work on more than one ranges in sequence may far outweigh the restriction of not being able to say something like that.

As an easy alternative, we could give --start-number=<n> to format-patch so that you can do the iteration yourself instead of having format-patch to iterate over the list.