On Sun, Mar 23, 2014 at 6:09 PM, Jeff Sipek <jeffpc@xxxxxxxxxxxxxx> wrote:
> On Fri, Mar 21, 2014 at 08:31:44AM +0100, Per Cederqvist wrote:
>> When extracting the patch, we only want the actual patches. We don't
>> want the "---" delimiter. Simplify the extraction by simply deleting
>> everything before the first "diff " line. (Use sed instead of awk to
>> simplify the code.)
>>
>> Without this patch, "guilt fold" and "guilt push" sometimes fails if
>> guilt.diffstat is true.
>>
>> Signed-off-by: Per Cederqvist <cederp@xxxxxxxxx>
>> ---
>> guilt | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/guilt b/guilt
>> index 8701481..c59cd0f 100755
>> --- a/guilt
>> +++ b/guilt
>> @@ -332,12 +332,7 @@ do_make_header()
>> # usage: do_get_patch patchfile
>> do_get_patch()
>> {
>> - awk '
>> -BEGIN{}
>> -/^(diff |---$|--- )/ {patch = 1}
>> -patch == 1 {print $0}
>> -END{}
>> -' < "$1"
>> + sed -n '/^diff /,$p' < "$1"
>
> So, the thought behind this mess was to allow minimal patches to work. The
> 'diff' line is *not* required by patch(1)!
I see. I can see that this is sometimes useful, even though I think
it is more important that guilt actually works with the patches itself
creates.
> Is it a problem if a patch description contains a line that starts with
> 'diff '? (The current code has this issue as well.)
Yes.
> For the record, this ambiguity is what's on several occasions made me
> consider splitting the header and the patch into separate files. Yeah, it'd
> be messier. :/
I really like having the header and the patches in the same file. I had
to solve a similar problem when I wrote my utility for diffing two diff
files (https://git.lysator.liu.se/pdiffdiff). Based on the experience I got
doing that, I propose this new do_get_patch function:
# usage: do_get_patch patchfile
do_get_patch()
{
awk '
BEGIN {
patchheader = ""
patch = 0
}
patch == 1 {
print $0
next
}
/^(diff |index |=|RCS file:|retrieving revision )/ {
patchheader = patchheader $0 "\n"
next
}
/^(--- |((new|deleted) file|old) mode )/ {
printf "%s", patchheader
-/^(diff |---$|--- )/ {patch = 1} print $0
patch = 1
}
{
patchheader = ""
}' < "$1"
}
The idea is to collect lines that *might* start the patch in
patchheader. Once we detect the start of the patch (via a
line that matches "--- " or any of the mode change lines)
we print the patcheader and the current line. If we get a
line that neither looks like a patchheader nor starts a patch,
we discard the patcheader. This is the case of a commit
message with a line that starts with "diff ".
The function above solves the issue with lines that
start with "diff " in the commit message. On the other
hand, it introduces the same issue for lines that start
with for instance "old mode ".
Actually, a smaller change that probably fixes the
issue with diffstat is to replace
/^(diff |---$|--- )/ {patch = 1}
witih
/^(diff |--- )/ {patch = 1}
as the problem with the original implementation is that
the "---\n" line that starts the diffstat should not start
the patch.
/ceder
>> }
>>
>> # usage: do_get_header patchfile
>> --
>> 1.8.3.1
>>
>
> --
> Defenestration n. (formal or joc.):
> The act of removing Windows from your computer in disgust, usually
> followed by the installation of Linux or some other Unix-like operating
> system.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html