twaugh / patchutils

Manipulate patch files
GNU General Public License v2.0
139 stars 22 forks source link

filterdiff: Fix header output. #13

Closed iazz closed 8 years ago

iazz commented 8 years ago

When outputting header lines, apply prefix changes only to lines starting with "---" and "+++" but also apply these changes to arguments of a possibly diff command line.

Adapt tests/fullheader4/run-test and remove it from XFAIL_TESTS.

This fixes #12.

iazz commented 8 years ago

On Thu, Sep 15, 2016 at 11:17:16AM -0700, thus spake Tim Waugh:

@twaugh approved this pull request.

This looks great, thanks! Added a couple of notes, nothing major though.


In [1]src/filterdiff.c:

  • fwrite (line, 1, 4, stdout);

  • if (prefix_to_add)
  • fputs (prefix_to_add, stdout);
  • else {
  • if (old_prefix_to_add && strncmp (line, "---", 3) == 0)
  • fputs (old_prefix_to_add, stdout);
  • if (new_prefix_to_add && strncmp (line, "+++", 3) == 0)
  • fputs (new_prefix_to_add, stdout);
  • }
  • if (strncmp(line, "diff", 4) == 0 && isspace(line[4])) {
  • size_t args = 0;
  • const char end = line + 5, begin = end, *ws = end;;

There's an extra semi-colon at the end of the line here.

Sure, my bad.

In [2]src/filterdiff.c:

  • fwrite (line, 1, 4, stdout);

  • if (prefix_to_add)
  • fputs (prefix_to_add, stdout);
  • else {
  • if (old_prefix_to_add && strncmp (line, "---", 3) == 0)
  • fputs (old_prefix_to_add, stdout);
  • if (new_prefix_to_add && strncmp (line, "+++", 3) == 0)
  • fputs (new_prefix_to_add, stdout);
  • }
  • if (strncmp(line, "diff", 4) == 0 && isspace(line[4])) {

The project style is to have a space between the function name and the opening parenthesis.

Oh yeah, these escaped my attention.

In [3]tests/fullheader4/run-test:

@@ -19,7 +19,7 @@ EOF ${FILTERDIFF} --strip=1 git-output 2>errors >output || { cat errors; exit 1; } [ -s errors ] && { cat errors; exit 1; } cmp output <<"EOF" || exit 1 -diff --git a/test.txt b/test.txt +diff --git test.txt test.txt

Oops, thanks. :)

I'm not 100% sure about the sense of this, as the resulting diff command-line looks really awkward, but I suppose it's a reasonable thing to do anyway.

Cheers,

Ignacy