Linus Torvalds [off-list ref] 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.
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.
On Tue, 3 Jul 2007, Paul Eggert wrote:
Linus Torvalds [off-list ref] writes:
quoted
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.
quoted
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
On Tue, 3 Jul 2007, Linus Torvalds wrote:
But maybe you can use this patch as a starting point, at least.
Oh, Paul - I forgot to mention, and since it wasn't necessarily clear from
the patch..
I used the patch-2.5.9 sources as a base for this. I don't know how
official that source base is, I picked it up from a Debian package as the
"original tar-ball": patch_2.5.9.orig.tar.gz
I suspect it applies to just about any version of patch with no
modifications, but just to clarify what the base was. Judging by the
ChangeLog, you're the one who has been doing the later 2.5.x releases even
though the last version on the GNU sites is 2.5.4.
Confusing.
Linus