[Quilt-dev] Re: being nice to patch(1)

[Linus Torvalds]

On Tue, 3 Jul 2007, Paul Eggert wrote:
>
> Linus Torvalds <address@hidden> writes:
> 
> > Anyway, I tried to look at the patch sources, but I had to stop. That 
> > whole "intuit_diff_type()" function is probably designed as an initiation 
> > rite for any patch programmers, and to make sure that you have to be 
> > really serious about wanting to send patches before you can become part of 
> > the "in crowd". It's "mental hazing".
> 
> You should have seen it in the good old days when Larry Wall wrote it.
> It was at least -- at least! -- 10% worse.

Heh.

Anyway, I figured out a sane way to do this - I had been thinking about it 
all wrong. Instead of worrying about all the places that change (and look 
at) "p_indent" - which is the real variable - I just made it not calculate 
"indent" in the first place.

That makes it catch it all in one place, and then quite naturally ignore 
any indented hunks because it won't recognize them as patches any more. At 
least I _think_ so, from just reading the source code.

So a patch like the following may or may not work. It compiles, but quite 
frankly, while it makes sense and worked for the single test-case I 
bothered with, somebody should double-check the logic.

> > In this case, the improvement would be to simply ignore indented patches 
> > (preferably by default, but at least have the option to do so).
> 
> I agree.  POSIX has tied our hands to some extent, though, since it
> _requires_ patch to accept indented patches by default.  It's too late
> to fix this in the current POSIX go-round, but we can fix it in the
> next.  And in the mean time we can add an option, I suppose defaulting
> to not stripping indentation unless POSIXLY_CORRECT is set.  That
> would be fine with me.
> 
> I'll add it to my list of things to do.

Ok, so this doesn't do the POSIXLY_CORRECT thing, and you may not agree 
with the flag name either ("--strip-indent" is a lot of characters to 
write, but I thought it was so esoteric that I think it's ok. I never even 
realized "patch" would ever do something as strange as that, and I'm 
hoping a lot of other people didn't realize either, so that changing this 
isn't going to matter, and very few people would hopefully ever need to 
use the "--strip-indent" flag).

But maybe you can use this patch as a starting point, at least.

(And if you wonder why I put the "if (!strip_indentation)" thing inside 
the loop, even though it doesn't ever change in the loop: it generated a 
smaller patch, and I didn't want to re-indent that code and make it even 
worse. So the logic is kind of stupid, but whatever..)

                Linus

---
diff --git a/common.h b/common.h
index c7fc5c2..45fecdc 100644
--- a/common.h
+++ b/common.h
@@ -174,6 +174,7 @@ XTERN bool canonicalize;
 XTERN int patch_get;
 XTERN bool set_time;
 XTERN bool set_utc;
+XTERN bool strip_indentation;
 
 enum diff
   {
diff --git a/patch.c b/patch.c
index 9e04daf..6e25d2a 100644
--- a/patch.c
+++ b/patch.c
@@ -522,6 +522,7 @@ static struct option const longopts[] =
   {"no-backup-if-mismatch", no_argument, NULL, CHAR_MAX + 6},
   {"posix", no_argument, NULL, CHAR_MAX + 7},
   {"quoting-style", required_argument, NULL, CHAR_MAX + 8},
+  {"strip-indent", no_argument, NULL, CHAR_MAX + 9},
   {NULL, no_argument, NULL, 0}
 };
 
@@ -580,6 +581,7 @@ static char const *const option_help[] =
 "  --verbose  Output extra information about the work being done.",
 "  --dry-run  Do not actually change any files; just print what would happen.",
 "  --posix  Conform to the POSIX standard.",
+"  --strip-indent handle indented patches.",
 "",
 "  -d DIR  --directory=DIR  Change the working directory to DIR first.",
 #if HAVE_SETMODE_DOS
@@ -779,6 +781,9 @@ get_some_switches (void)
                                     (enum quoting_style) i);
                }
                break;
+           case CHAR_MAX + 9: /* --strip-indent */
+               strip_indentation = true;
+               break;
            default:
                usage (stderr, 2);
        }
diff --git a/pch.c b/pch.c
index d98af86..fcb08c5 100644
--- a/pch.c
+++ b/pch.c
@@ -345,6 +345,8 @@ intuit_diff_type (void)
        }
        strip_trailing_cr = 2 <= chars_read && buf[chars_read - 2] == '\r';
        for (s = buf; *s == ' ' || *s == '\t' || *s == 'X'; s++) {
+           if (!strip_indentation)
+               break;
            if (*s == '\t')
                indent = (indent + 8) & ~7;
            else