[RFC] Add a suffix option to git-format-patch

[RFC] Add a suffix option to git-format-patch

[Josh Boyer]

Hi All,

I use git quite a bit to track my changes and then use
git-format-patch to generate patches to send on to others.  For the
most part, it works great but I find myself constantly doing:

mv xxxx-foo.txt xxxx-foo.patch

Could we add an option to git-format-patch to use ".patch" as the file
suffix instead of ".txt"?  Something like the below?

josh

diff --git a/builtin-log.c b/builtin-log.c
index a59b4ac..4eb2d32 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -217,6 +217,7 @@ static int git_format_config(const char *var,
const char *value)

 static FILE *realstdout = NULL;
 static const char *output_directory = NULL;
+static int psuffix = 0;

 static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 {
@@ -265,7 +266,11 @@ static void reopen_stdout(struct commit *commit,
int nr, int keep_subject)
 		while (filename[len - 1] == '.' || filename[len - 1] == '-')
 			len--;
 	}
-	strcpy(filename + len, ".txt");
+
+	if (psuffix)
+		strcpy(filename + len, ".patch");
+	else
+		strcpy(filename + len, ".txt");
 	fprintf(realstdout, "%s\n", filename);
 	freopen(filename, "w", stdout);
 }
@@ -436,6 +441,8 @@ int cmd_format_patch(int argc, const char **argv,
const char *prefix)
 				die("Need a Message-Id for --in-reply-to");
 			in_reply_to = argv[i];
 		}
+		else if (!strcmp(argv[i], "--psuffix"))
+			psuffix = 1;
 		else
 			argv[j++] = argv[i];
 	}

Re: [RFC] Add a suffix option to git-format-patch

[Johannes Schindelin]

Hi,

On Wed, 17 Jan 2007, Josh Boyer wrote:

> Could we add an option to git-format-patch to use ".patch" as the file
> suffix instead of ".txt"?  Something like the below?
> 
> diff --git a/builtin-log.c b/builtin-log.c
> index a59b4ac..4eb2d32 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -217,6 +217,7 @@ static int git_format_config(const char *var,
> const char *value)
> 
> static FILE *realstdout = NULL;
> static const char *output_directory = NULL;
> +static int psuffix = 0;

Why not

	static const char *file_extension = ".txt"

Hmm?

> 	}
> -	strcpy(filename + len, ".txt");

Here, you would write

	strcpy(filename + len, file_extension);

> @@ -436,6 +441,8 @@ int cmd_format_patch(int argc, const char **argv,
> const char *prefix)
> 				die("Need a Message-Id for --in-reply-to");
> 			in_reply_to = argv[i];
> 		}
> +		else if (!strcmp(argv[i], "--psuffix"))
> +			psuffix = 1;

and here:

		else if (!strncmp(argv[i], "--extension=", 12))
			file_extension = argv[i] + 12;

You'd call it with "--extension=.patch", and if you really want, you can 
make a config variable from it.

Ciao,
Dscho

Re: [RFC] Add a suffix option to git-format-patch

[Josh Boyer]

On 1/17/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Wed, 17 Jan 2007, Josh Boyer wrote:
>
> > Could we add an option to git-format-patch to use ".patch" as the file
> > suffix instead of ".txt"?  Something like the below?
> >
> > diff --git a/builtin-log.c b/builtin-log.c
> > index a59b4ac..4eb2d32 100644
> > --- a/builtin-log.c
> > +++ b/builtin-log.c
> > @@ -217,6 +217,7 @@ static int git_format_config(const char *var,
> > const char *value)
> >
> > static FILE *realstdout = NULL;
> > static const char *output_directory = NULL;
> > +static int psuffix = 0;
>
> Why not
>
>         static const char *file_extension = ".txt"
>
> Hmm?

Yes, that's better.  I was more going for "would something like this
option be accepted" to start with.  And I'm lazy, so the patch I wrote
was just an example for my specific use case :).

> You'd call it with "--extension=.patch", and if you really want, you can
> make a config variable from it.

Good idea.  I'll try and get this done soon-ish.  I'm not very
familiar with the git code, so if someone beats me to it, I won't be
disappointed in the least ;)

josh

Re: [RFC] Add a suffix option to git-format-patch

[Horst H. von Brand]

Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> On Wed, 17 Jan 2007, Josh Boyer wrote:
> 
> > Could we add an option to git-format-patch to use ".patch" as the file
> > suffix instead of ".txt"?  Something like the below?
> > 
> > diff --git a/builtin-log.c b/builtin-log.c
> > index a59b4ac..4eb2d32 100644
> > --- a/builtin-log.c
> > +++ b/builtin-log.c
> > @@ -217,6 +217,7 @@ static int git_format_config(const char *var,
> > const char *value)
> > 
> > static FILE *realstdout = NULL;
> > static const char *output_directory = NULL;
> > +static int psuffix = 0;
> 
> Why not
> 
> 	static const char *file_extension = ".txt"

Need to keep the length of that around for allocating string space and
such. Is the flexibility worth the hassle?
-- 
Dr. Horst H. von Brand                   User #22616 counter.li.org
Departamento de Informatica                    Fono: +56 32 2654431
Universidad Tecnica Federico Santa Maria             +56 32 2654239
Casilla 110-V, Valparaiso, Chile               Fax:  +56 32 2797513

[PATCH] Introduce 'git-format-patch --suffix=patch'

[Junio C Hamano]

The default can also be changed with "format.suffix" configuration.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

  Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

  > Why not
  >
  > 	static const char *file_extension = ".txt"
  >
  > Hmm?

  Let's do this instead.  By the way, there is a bug in the
  configuration parsing for format.headers from commit 20ff0680,
  which needs to be check NULLness of the value the same way as
  this one deals with format.suffix, which I've already fixed in
  my tree.

 Documentation/git-format-patch.txt |   13 ++++++++++++-
 builtin-log.c                      |   19 ++++++++++++++++---
 2 files changed, 28 insertions(+), 4 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index 67425dc..34abd2f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 [verse]
 'git-format-patch' [-n | -k] [-o <dir> | --stdout] [--attach] [--thread]
 	           [-s | --signoff] [--diff-options] [--start-number <n>]
-		   [--in-reply-to=Message-Id]
+		   [--in-reply-to=Message-Id] [--suffix=<sfx>]
 		   <since>[..<until>]
 
 DESCRIPTION
@@ -78,6 +78,12 @@ OPTIONS
 	reply to the given Message-Id, which avoids breaking threads to
 	provide a new patch series.
 
+--suffix=<sfx>::
+	Instead of using `txt` as the suffix for generated
+	filenames, use specifed suffix.  A common alternative is
+	`--suffix=patch`.
+
+
 CONFIGURATION
 -------------
 You can specify extra mail header lines to be added to each
@@ -86,6 +92,11 @@ message in the repository configuration as follows:
 [format]
         headers = "Organization: git-foo\n"
 
+You can specify default suffix used:
+
+[format]
+        suffix = patch
+
 
 EXAMPLES
 --------
diff --git a/builtin-log.c b/builtin-log.c
index a59b4ac..04e3144 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -197,6 +197,7 @@ static int istitlechar(char c)
 
 static char *extra_headers = NULL;
 static int extra_headers_size = 0;
+static const char *fmt_patch_suffix = "txt";
 
 static int git_format_config(const char *var, const char *value)
 {
@@ -208,6 +209,12 @@ static int git_format_config(const char *var, const char *value)
 		strcat(extra_headers, value);
 		return 0;
 	}
+	if (!strcmp(var, "format.suffix")) {
+		if (!value)
+			die("format.suffix without value");
+		fmt_patch_suffix = xstrdup(value);
+		return 0;
+	}
 	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
 		return 0;
 	}
@@ -223,9 +230,10 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 	char filename[1024];
 	char *sol;
 	int len = 0;
+	int suffix_len = strlen(fmt_patch_suffix) + 10; /* ., NUL and slop */
 
 	if (output_directory) {
-		strlcpy(filename, output_directory, 1010);
+		strlcpy(filename, output_directory, 1000);
 		len = strlen(filename);
 		if (filename[len - 1] != '/')
 			filename[len++] = '/';
@@ -249,7 +257,10 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 			}
 		}
 
-		for (j = 0; len < 1024 - 6 && sol[j] && sol[j] != '\n'; j++) {
+		for (j = 0;
+		     len < sizeof(filename) - suffix_len &&
+			     sol[j] && sol[j] != '\n';
+		     j++) {
 			if (istitlechar(sol[j])) {
 				if (space) {
 					filename[len++] = '-';
@@ -265,7 +276,7 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 		while (filename[len - 1] == '.' || filename[len - 1] == '-')
 			len--;
 	}
-	strcpy(filename + len, ".txt");
+	sprintf(filename + len, ".%s", fmt_patch_suffix);
 	fprintf(realstdout, "%s\n", filename);
 	freopen(filename, "w", stdout);
 }
@@ -436,6 +447,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 				die("Need a Message-Id for --in-reply-to");
 			in_reply_to = argv[i];
 		}
+		else if (!strncmp(argv[i], "--suffix=", 9))
+			fmt_patch_suffix = argv[i] + 9;
 		else
 			argv[j++] = argv[i];
 	}
-- 
1.5.0.rc1.g5e1a2-dirty

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Andy Whitcroft]

Junio C Hamano wrote:
> The default can also be changed with "format.suffix" configuration.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>
> ---
> 
>   Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
>   > Why not
>   >
>   > 	static const char *file_extension = ".txt"
>   >
>   > Hmm?
> 
>   Let's do this instead.  By the way, there is a bug in the
>   configuration parsing for format.headers from commit 20ff0680,
>   which needs to be check NULLness of the value the same way as
>   this one deals with format.suffix, which I've already fixed in
>   my tree.
> 
>  Documentation/git-format-patch.txt |   13 ++++++++++++-
>  builtin-log.c                      |   19 ++++++++++++++++---
>  2 files changed, 28 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index 67425dc..34abd2f 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -11,7 +11,7 @@ SYNOPSIS
>  [verse]
>  'git-format-patch' [-n | -k] [-o <dir> | --stdout] [--attach] [--thread]
>  	           [-s | --signoff] [--diff-options] [--start-number <n>]
> -		   [--in-reply-to=Message-Id]
> +		   [--in-reply-to=Message-Id] [--suffix=<sfx>]
>  		   <since>[..<until>]
>  
>  DESCRIPTION
> @@ -78,6 +78,12 @@ OPTIONS
>  	reply to the given Message-Id, which avoids breaking threads to
>  	provide a new patch series.
>  
> +--suffix=<sfx>::
> +	Instead of using `txt` as the suffix for generated
> +	filenames, use specifed suffix.  A common alternative is
> +	`--suffix=patch`.
> +
> +
>  CONFIGURATION
>  -------------
>  You can specify extra mail header lines to be added to each
> @@ -86,6 +92,11 @@ message in the repository configuration as follows:
>  [format]
>          headers = "Organization: git-foo\n"
>  
> +You can specify default suffix used:
> +
> +[format]
> +        suffix = patch
> +
>  
>  EXAMPLES
>  --------
> diff --git a/builtin-log.c b/builtin-log.c
> index a59b4ac..04e3144 100644
> --- a/builtin-log.c
> +++ b/builtin-log.c
> @@ -197,6 +197,7 @@ static int istitlechar(char c)
>  
>  static char *extra_headers = NULL;
>  static int extra_headers_size = 0;
> +static const char *fmt_patch_suffix = "txt";
>  
>  static int git_format_config(const char *var, const char *value)
>  {
> @@ -208,6 +209,12 @@ static int git_format_config(const char *var, const char *value)
>  		strcat(extra_headers, value);
>  		return 0;
>  	}
> +	if (!strcmp(var, "format.suffix")) {
> +		if (!value)
> +			die("format.suffix without value");
> +		fmt_patch_suffix = xstrdup(value);
> +		return 0;
> +	}
>  	if (!strcmp(var, "diff.color") || !strcmp(var, "color.diff")) {
>  		return 0;
>  	}
> @@ -223,9 +230,10 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
>  	char filename[1024];
>  	char *sol;
>  	int len = 0;
> +	int suffix_len = strlen(fmt_patch_suffix) + 10; /* ., NUL and slop */
>  
>  	if (output_directory) {
> -		strlcpy(filename, output_directory, 1010);
> +		strlcpy(filename, output_directory, 1000);
>  		len = strlen(filename);
>  		if (filename[len - 1] != '/')
>  			filename[len++] = '/';
> @@ -249,7 +257,10 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
>  			}
>  		}
>  
> -		for (j = 0; len < 1024 - 6 && sol[j] && sol[j] != '\n'; j++) {
> +		for (j = 0;
> +		     len < sizeof(filename) - suffix_len &&
> +			     sol[j] && sol[j] != '\n';
> +		     j++) {
>  			if (istitlechar(sol[j])) {
>  				if (space) {
>  					filename[len++] = '-';
> @@ -265,7 +276,7 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
>  		while (filename[len - 1] == '.' || filename[len - 1] == '-')
>  			len--;
>  	}
> -	strcpy(filename + len, ".txt");
> +	sprintf(filename + len, ".%s", fmt_patch_suffix);

This doesn't give us any possibility of not having a suffix.  Can we not
include the . in the suffix here so that we can specify it as "".

>  	fprintf(realstdout, "%s\n", filename);
>  	freopen(filename, "w", stdout);
>  }
> @@ -436,6 +447,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  				die("Need a Message-Id for --in-reply-to");
>  			in_reply_to = argv[i];
>  		}
> +		else if (!strncmp(argv[i], "--suffix=", 9))
> +			fmt_patch_suffix = argv[i] + 9;
>  		else
>  			argv[j++] = argv[i];
>  	}

-apw

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Junio C Hamano]

Andy Whitcroft <apw@shadowen.org> writes:

>> -	strcpy(filename + len, ".txt");
>> +	sprintf(filename + len, ".%s", fmt_patch_suffix);
>
> This doesn't give us any possibility of not having a suffix.  Can we not
> include the . in the suffix here so that we can specify it as "".

I've considered it, but I do not think it is worth it.  

If we did so, the configuration would look like:

	[format]
		suffix = .txt

which has a certain "Huh?" factor, and more importantly, a
careless user would end up with a patchfile that is named:

	0001-Introduce-git-format-patch-suffix-patchtxt

which is I think much worse than not being able to say:

	0001-Introduce-git-format-patch-suffix-patch

But I do not care that much either way.

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Brian Gernhardt]

On Jan 17, 2007, at 2:27 PM, Junio C Hamano wrote:

> Andy Whitcroft <apw@shadowen.org> writes:
>
>>> -	strcpy(filename + len, ".txt");
>>> +	sprintf(filename + len, ".%s", fmt_patch_suffix);
>>
>> This doesn't give us any possibility of not having a suffix.  Can  
>> we not
>> include the . in the suffix here so that we can specify it as "".
>
> I've considered it, but I do not think it is worth it.

I think that the best form of DWIM is that if the suffix is "", then  
you simply skip the entire sprintf.  Then any suffix has to have the  
'.', but no suffix doesn't have it.  Additional DWIMMmery could  
remove an initial '.' from the suffix so that users expecting it to  
be there don't get ".." in their file.

Patch (on top of yours) should follow shortly.

~~ Brian

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Junio C Hamano]

Brian Gernhardt <benji@silverinsanity.com> writes:

> I think that the best form of DWIM is that if the suffix is "", then
> you simply skip the entire sprintf.  Then any suffix has to have the
> '.', but no suffix doesn't have it.  Additional DWIMMmery could
> remove an initial '.' from the suffix so that users expecting it to
> be there don't get ".." in their file.

I think it is generally accepted on this list that that kind of
DWIMmery is bad.

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Brian Gernhardt]

On Jan 17, 2007, at 2:57 PM, Junio C Hamano wrote:

> Brian Gernhardt <benji@silverinsanity.com> writes:
>
>> I think that the best form of DWIM is that if the suffix is "", then
>> you simply skip the entire sprintf.  Then any suffix has to have the
>> '.', but no suffix doesn't have it.  Additional DWIMMmery could
>> remove an initial '.' from the suffix so that users expecting it to
>> be there don't get ".." in their file.
>
> I think it is generally accepted on this list that that kind of
> DWIMmery is bad.

The first part seems like the easiest way to allow --suffix=""  
actually remove the suffix while removing the "Huh?" factor you  
mentioned.  In fact, having --suffix="" add a suffix of "." is  
somewhat confusing all on it's own.

The second just seems like an easy way to keep stupid mistakes from  
occurring, but isn't needed.

I'll still post the patch, but not add the extra DWIMmery.  I think  
it's a better alternative than having to choose between putting "."  
in all the suffixes and always having "." on the file.

~~ Brian

[PATCH] Make format-patch --suffix="" not add any suffix

[Brian Gernhardt]

Having a suffix of "." when the user explicitly asked for none is
somewhat confusing.

Signed-off-by: Brian Gernhardt <benji@silverinsanity.com>
---
 builtin-log.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/builtin-log.c b/builtin-log.c
index 04e3144..a60a987 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -276,7 +276,8 @@ static void reopen_stdout(struct commit *commit, int nr, int keep_subject)
 		while (filename[len - 1] == '.' || filename[len - 1] == '-')
 			len--;
 	}
-	sprintf(filename + len, ".%s", fmt_patch_suffix);
+	if (fmt_patch_suffix[0] != '\0')
+		sprintf(filename + len, ".%s", fmt_patch_suffix);
 	fprintf(realstdout, "%s\n", filename);
 	freopen(filename, "w", stdout);
 }
-- 
1.5.0.rc1.gfb138

Re: [PATCH] Introduce 'git-format-patch --suffix=patch'

[Johannes Schindelin]

Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de>

Re: [RFC] Add a suffix option to git-format-patch

[David Kågedal]

"Josh Boyer" <jwboyer@gmail.com> writes:

> Hi All,
>
> I use git quite a bit to track my changes and then use
> git-format-patch to generate patches to send on to others.  For the
> most part, it works great but I find myself constantly doing:
>
> mv xxxx-foo.txt xxxx-foo.patch

Seconded. I would even prefer .patch to be default, but I guess a
config parameter would help me there.

-- 
David Kågedal

Re: [RFC] Add a suffix option to git-format-patch

[Andreas Ericsson]

David Kågedal wrote:
> "Josh Boyer" <jwboyer@gmail.com> writes:
> 
>> Hi All,
>>
>> I use git quite a bit to track my changes and then use
>> git-format-patch to generate patches to send on to others.  For the
>> most part, it works great but I find myself constantly doing:
>>
>> mv xxxx-foo.txt xxxx-foo.patch
> 
> Seconded. I would even prefer .patch to be default, but I guess a
> config parameter would help me there.
> 

Thirded, although I'd rather have it .gpatch, as it's a full commit with 
message and all. It isn't necessarily usable with the 'patch' program 
without manual massage first.

I can live with .txt though. Not many other files I keep around have 
names quite like 
0001-loadbalance_module-support-config-fanout-in-static-mesh.txt, so 
it's not as if I frequently confuse them.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [RFC] Add a suffix option to git-format-patch

[Johannes Schindelin]

Hi,

On Wed, 17 Jan 2007, Andreas Ericsson wrote:

> It [a git patch] isn't necessarily usable with the 'patch' program 
> without manual massage first.

All "patch" programs I know are happy without any massage.

Ciao,
Dscho

Re: [RFC] Add a suffix option to git-format-patch

[Junio C Hamano]

David Kågedal <davidk@lysator.liu.se> writes:

> "Josh Boyer" <jwboyer@gmail.com> writes:
>
>> I use git quite a bit to track my changes and then use
>> git-format-patch to generate patches to send on to others.  For the
>> most part, it works great but I find myself constantly doing:
>>
>> mv xxxx-foo.txt xxxx-foo.patch
>
> Seconded. I would even prefer .patch to be default, but I guess a
> config parameter would help me there.

Two minor objections to changing the default are: (1) it's a
change and any change is bad ;-) and (2) the reason I changed it
to .txt before submitting the original format-patch to Linus was
because Emacs wanted to go into its "diff" mode when files are
named with .patch suffix, which had two annoyances (read-only by
default, and editing patch tried to automatically recount diff
and its recounting screwed up in some cases I do not remember
the details about).

I do not have a problem with a config or an option, though.

Re: [RFC] Add a suffix option to git-format-patch

[David Kågedal]

Junio C Hamano <junkio@cox.net> writes:

> Emacs wanted to go into its "diff" mode when files are named with
> .patch suffix, 

This is probably the primary reason why I *want* it to be .patch.  The
diff-mode in Emacs is really useful, and read-write mode is only a C-x
C-q away.

> which had two annoyances (read-only by default, and
> editing patch tried to automatically recount diff and its recounting
> screwed up in some cases I do not remember the details about).

I rarely edit patches, but it has worked for me.

-- 
David Kågedal

Re: [RFC] Add a suffix option to git-format-patch

[Josh Boyer]

On 1/17/07, Junio C Hamano <junkio@cox.net> wrote:
> David Kågedal <davidk@lysator.liu.se> writes:
>
> > "Josh Boyer" <jwboyer@gmail.com> writes:
> >
> >> I use git quite a bit to track my changes and then use
> >> git-format-patch to generate patches to send on to others.  For the
> >> most part, it works great but I find myself constantly doing:
> >>
> >> mv xxxx-foo.txt xxxx-foo.patch
> >
> > Seconded. I would even prefer .patch to be default, but I guess a
> > config parameter would help me there.
>
> Two minor objections to changing the default are: (1) it's a
> change and any change is bad ;-) and (2) the reason I changed it
> to .txt before submitting the original format-patch to Linus was
> because Emacs wanted to go into its "diff" mode when files are
> named with .patch suffix, which had two annoyances (read-only by
> default, and editing patch tried to automatically recount diff
> and its recounting screwed up in some cases I do not remember
> the details about).

Well there's your problem.  You're using Emacs.  ;)

Seriously though, I'll try and fix up the patch a bit soon and resubmit.

josh

Re: [RFC] Add a suffix option to git-format-patch

[Josh Boyer]

On 1/17/07, Josh Boyer <jwboyer@gmail.com> wrote:
>
> Seriously though, I'll try and fix up the patch a bit soon and resubmit.

I see Junio beat me to it.  Excellent :)

josh

[PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Junio C Hamano]

Editors often give easier handling of patch files if the
filename ends with .patch, so use it instead of .txt.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

  Junio C Hamano <junkio@cox.net> writes:

  > "Josh Boyer" <jwboyer@gmail.com> writes:
  >
  >> On 1/17/07, Junio C Hamano <junkio@cox.net> wrote:
  >>
  >>> Two minor objections to changing the default are: (1) it's a
  >>> change and any change is bad ;-) and (2) the reason I changed it
  >>> to .txt before submitting the original format-patch to Linus was
  >>> because Emacs wanted to go into its "diff" mode when files are
  >>> named with .patch suffix, which had two annoyances (read-only by
  >>> default, and editing patch tried to automatically recount diff
  >>> and its recounting screwed up in some cases I do not remember
  >>> the details about).
  >>
  >> Well there's your problem.  You're using Emacs.  ;)
  >
  > Fair enough.  Its probably that there is something wrong in the
  > way I am using Emacs diff/patch editing mode.  Even if the
  > problem I had were because of bugs in Emacs, the users of git
  > should not have to suffer from "unusual" suffixes to work them
  > around.
  >
  > So that lifts one of the objections.  What should be done to the
  > other one --- time for a quick poll?

 Documentation/git-format-patch.txt                 |   15 +++++++--------
 .../howto/rebase-from-internal-branch.txt          |    2 +-
 builtin-log.c                                      |    2 +-
 3 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index c0ffe99..49f51bb 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -79,9 +79,9 @@ OPTIONS
 	provide a new patch series.
 
 --suffix=.<sfx>::
-	Instead of using `.txt` as the suffix for generated
+	Instead of using `.patch` as the suffix for generated
 	filenames, use specifed suffix.  A common alternative is
-	`--suffix=.patch`.
+	`--suffix=.txt`.
 +
 Note that you would need to include the leading dot `.` if you
 want a filename like `0001-description-of-my-change.patch`, and
@@ -91,15 +91,14 @@ not add any suffix.
 CONFIGURATION
 -------------
 You can specify extra mail header lines to be added to each
-message in the repository configuration as follows:
+message in the repository configuration.  Also you can specify
+the default suffix different from the built-in one:
 
+------------
 [format]
         headers = "Organization: git-foo\n"
-
-You can specify default suffix used:
-
-[format]
-        suffix = .patch
+        suffix = .txt
+------------
 
 
 EXAMPLES
diff --git a/Documentation/howto/rebase-from-internal-branch.txt b/Documentation/howto/rebase-from-internal-branch.txt
index fcd64e9..3b3a5c2 100644
--- a/Documentation/howto/rebase-from-internal-branch.txt
+++ b/Documentation/howto/rebase-from-internal-branch.txt
@@ -106,7 +106,7 @@ prepare #2 and #3 for e-mail submission.
 
     $ git format-patch master^^ master
 
-This creates two files, 0001-XXXX.txt and 0002-XXXX.txt.  Send
+This creates two files, 0001-XXXX.patch and 0002-XXXX.patch.  Send
 them out "To: " your project maintainer and "Cc: " your mailing
 list.  You could use contributed script git-send-email if
 your host has necessary perl modules for this, but your usual
diff --git a/builtin-log.c b/builtin-log.c
index 930cc04..f3cff13 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -197,7 +197,7 @@ static int istitlechar(char c)
 
 static char *extra_headers = NULL;
 static int extra_headers_size = 0;
-static const char *fmt_patch_suffix = ".txt";
+static const char *fmt_patch_suffix = ".patch";
 
 static int git_format_config(const char *var, const char *value)
 {
-- 
1.5.0.rc1.gde38

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

since this is a poll, count me as neutral. I don't care either way.

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Junio C Hamano <junkio@cox.net> wrote:
> Editors often give easier handling of patch files if the
> filename ends with .patch, so use it instead of .txt.

I'd like to see ".patch" there, but...

I have to mention, though, that the majority of the editing
programs is used on that stupid thing called windows
and they usually and most notably have no idea what
the patch is and how could it happen a file extension
having more than 3 characters. I am already having
hard time explaining to my coworkers what the patch
(the program and the file) is. With mixed success.

Also, how many mail clients know that .patch is actually
a text and not application/binary? It'll make patch
reviewing harder for some (not sure if I'd like a review
of such a person, though).

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> Also, how many mail clients know that .patch is actually
> a text and not application/binary? It'll make patch
> reviewing harder for some (not sure if I'd like a review
> of such a person, though).

Patches intended for review should be sent inline, not attached.
Thus the file extension has no impact on how the mail client should
treat it.

Don't count people out just because they cannot read a *.patch file
All constructive feedback is valuable, no matter its source.  Of
course I did qualify that with "constructive"... ;-)

-- 
Shawn.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Alex Riesen <raa.lkml@gmail.com> wrote:
> > Also, how many mail clients know that .patch is actually
> > a text and not application/binary? It'll make patch
> > reviewing harder for some (not sure if I'd like a review
> > of such a person, though).
>
> Patches intended for review should be sent inline, not attached.

There is again this word "should". Are you sure it has any _real_
meaning? For a person who knows about revision management
from something like Perforce?

> Thus the file extension has no impact on how the mail client should
> treat it.

He will attach it. It's typical for outlook users. He will even put it
in HTML-formatted mail, because that's the default format for
outlook messages.

> Don't count people out just because they cannot read a *.patch file

I don't. I just know how hard is it to explain what source is and
why it is better than a "C++ file".

> All constructive feedback is valuable, no matter its source.  Of
> course I did qualify that with "constructive"... ;-)

It is. That's why I did try to explain it to some. That's how I know
about explaining. I'm very pessimistic now, sorry.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> On 1/18/07, Shawn O. Pearce <spearce@spearce.org> wrote:

>> Thus the file extension has no impact on how the mail client should
>> treat it.
>
> He will attach it. It's typical for outlook users.

If that is the case, I highly suspect that it is one more reason
not to mark the file with .txt; Outlook may say "Hey, it's TEXT,
so let's linewrap it, quote-balance it and add all sorts of nice
frills to make it easier to read for human consumption".  

Assuming that it does not understand what a .patch is, we would
have a better chance to force it not to look at nor touch the
contents, and not getting our patches corrupted.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Junio C Hamano <junkio@cox.net> wrote:
> >> Thus the file extension has no impact on how the mail client should
> >> treat it.
> >
> > He will attach it. It's typical for outlook users.
>
> If that is the case, I highly suspect that it is one more reason
> not to mark the file with .txt; Outlook may say "Hey, it's TEXT,
> so let's linewrap it, quote-balance it and add all sorts of nice
> frills to make it easier to read for human consumption".

No, it wont (and doesn't). It would be "too clever to be useful".
Outlook is too _stupid_ to be useful.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Junio C Hamano]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> I'd like to see ".patch" there, but...
>
> I have to mention, though, that the majority of the editing
> programs is used on that stupid thing called windows ...

Even if majority of git target audience were on Windows, I
thought majority of Windows users are on either VFAT or NTFS and
not DOS 8.3 filesystems these days.

> Also, how many mail clients know that .patch is actually
> a text and not application/binary? It'll make patch
> reviewing harder for some (not sure if I'd like a review
> of such a person, though).

Is it common for popular MUAs to have a single command that lets
you specify a file and depending on its suffix paste it inline
or make it an attachment?  I had an impression that most have
separate commands for "read text from file (as opposed to
typing)" and "attach a file (of random type, not necessarily and
more often than not text)".

The output of format-patch is not meant to be used as an
attachment (it is "read text from file" kind), so I do not think
your worry applies here.  Maybe something I am missing?

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Junio C Hamano <junkio@cox.net> wrote:
>
> > I'd like to see ".patch" there, but...
> >
> > I have to mention, though, that the majority of the editing
> > programs is used on that stupid thing called windows ...
>
> Even if majority of git target audience were on Windows, I
> thought majority of Windows users are on either VFAT or NTFS and
> not DOS 8.3 filesystems these days.

The filesystems are not 8.3, the programs are.

> > Also, how many mail clients know that .patch is actually
> > a text and not application/binary? It'll make patch
> > reviewing harder for some (not sure if I'd like a review
> > of such a person, though).
>
> Is it common for popular MUAs to have a single command that lets
> you specify a file and depending on its suffix paste it inline
> or make it an attachment?  I had an impression that most have
> separate commands for "read text from file (as opposed to
> typing)" and "attach a file (of random type, not necessarily and
> more often than not text)".

No, they don't :) They only have "attach" and drag-drop (which does the same).

> The output of format-patch is not meant to be used as an
> attachment (it is "read text from file" kind), so I do not think
> your worry applies here.  Maybe something I am missing?

Yes, the experience being a damned corporate windows user
in a novell netware network with 50-year old admin fixated on
microsoft exchange, not to mention Outlook Express users...

I think we'd raising the entry barrier with choosing the defaults
being so convenient for us. Well, the real-life programmers
are less of Unix-liking kind. They are more lazy and demotivated
kind, and Git will be _forced_ on them. It almost certainly
will not be their choice. Not always, some'll like it (heck, I know
people who swear by Perforce!), but most have a job, source
of income, and not the profession (like in professional pride).

As much as like Unix and everything related, I think it is
not reasonable to try to change the majority. Not unless
we have something earth-shattering. Well, git is, but
0001-fix....patch in email attachment probably not.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Josh Boyer]

On 1/18/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> I think we'd raising the entry barrier with choosing the defaults
> being so convenient for us. Well, the real-life programmers
> are less of Unix-liking kind. They are more lazy and demotivated
> kind, and Git will be _forced_ on them. It almost certainly
> will not be their choice. Not always, some'll like it (heck, I know
> people who swear by Perforce!), but most have a job, source
> of income, and not the profession (like in professional pride).

real-life programmers?  Please don't generalize.  It's insulting.

> As much as like Unix and everything related, I think it is
> not reasonable to try to change the majority. Not unless
> we have something earth-shattering. Well, git is, but
> 0001-fix....patch in email attachment probably not.

I would venture to say that the _majority_ of git users are not using
Windows.  In this enviroment, Linux is likely the dominant OS,
followed by other *nix.  So changing the extention to benefit the
majorit of _git's_ users is a good thing.

josh

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Josh Boyer wrote:

> real-life programmers?  Please don't generalize.  It's insulting.

What? Programmers with a real life? Don't be silly ;-)

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
>
> > real-life programmers?  Please don't generalize.  It's insulting.
>
> What? Programmers with a real life? Don't be silly ;-)
>

I know a married drinking black-belt doing Delphi.
He say he has "programmer" written all over him.
We saw to it once, even :)

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Josh Boyer <jwboyer@gmail.com> wrote:
> > I think we'd raising the entry barrier with choosing the defaults
> > being so convenient for us. Well, the real-life programmers
> > are less of Unix-liking kind. They are more lazy and demotivated
> > kind, and Git will be _forced_ on them. It almost certainly
> > will not be their choice. Not always, some'll like it (heck, I know
> > people who swear by Perforce!), but most have a job, source
> > of income, and not the profession (like in professional pride).
>
> real-life programmers?  Please don't generalize.  It's insulting.

Just because you are not able to realize that the count of
windows-based projects is still superior to that of say...
"likable" systems, they wont magically disappear.
If that is the case, I think I do some more insulting
and say that they are not only real-life but also will never
accept the statement of their ways being wrong, however
stupid they may appear. That's just how it is, count them if
you don't believe me.

> > As much as like Unix and everything related, I think it is
> > not reasonable to try to change the majority. Not unless
> > we have something earth-shattering. Well, git is, but
> > 0001-fix....patch in email attachment probably not.
>
> I would venture to say that the _majority_ of git users are not using
> Windows.

The _real_ majority of the programmers desperately need a better
VCS than CVS, SVN, Perforce, SourceSafe, ClearCase, etc.

> In this enviroment, Linux is likely the dominant OS,
> followed by other *nix.  So changing the extention to benefit the
> majorit of _git's_ users is a good thing.

Yes. For me and you. One of my coworkers knows nothing about patches,
but wants (and perfectly able to) review my code. He has usable brains
and is able to figure out what "+" and "-" is (he has, by now). He hasn't
even realized that it was an automatically generated information, as
I sent a patch to him first time, thought it was just a funny way to
document changes (and was surprised when I told him a patch can be
applied automatically, even if the original file is not exactly the same).
But he is a typical windows-trained programmer. Lazy, unmotivated and
happily married. He does programming by accident (was smart enough
to learn the basics of the trade). Why would he want to take the an
extra step of figuring out what that strange "0001-...patch" means?
Now, I know him, would never think about sending him a real patch.
I'm kinda grown and tired, and need the bastard, too. Someone younger
will just call him idiot and "improve" the situation by telling him about
"stupid windows" and "he should the right ways". Just to be answered
"it worked for some millions programmers before you" and "I told your
manager you making problems and are hard to communicate with".

People often understand "funny ways" the others may have. They
don't like been told they are wrong or stupid (especially when they
actually are stupid).

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Andreas Ericsson]

Alex Riesen wrote:
> On 1/18/07, Josh Boyer <jwboyer@gmail.com> wrote:
>> > As much as like Unix and everything related, I think it is
>> > not reasonable to try to change the majority. Not unless
>> > we have something earth-shattering. Well, git is, but
>> > 0001-fix....patch in email attachment probably not.
>>
>> I would venture to say that the _majority_ of git users are not using
>> Windows.
> 
> The _real_ majority of the programmers desperately need a better
> VCS than CVS, SVN, Perforce, SourceSafe, ClearCase, etc.
> 

They're free to use git ofcourse, provided they install cygwin or help 
migrate it to run natively on windows. I don't think anyone would cry if 
a competent cross-platform programmer stepped up and started submitting 
patches to get git working on windows without having to resort to the 
cygwin emulation layer.

The thing is, no-one's getting paid for it, so until someone *does* step 
up, it won't happen, as 95% of the *current* git users are still running 
what we on this list will indefinitely refer to as "a sane OS".

>> In this enviroment, Linux is likely the dominant OS,
>> followed by other *nix.  So changing the extention to benefit the
>> majorit of _git's_ users is a good thing.
> 
> Yes. For me and you. One of my coworkers knows nothing about patches,
> but wants (and perfectly able to) review my code. He has usable brains
> and is able to figure out what "+" and "-" is (he has, by now). He hasn't
> even realized that it was an automatically generated information, as
> I sent a patch to him first time, thought it was just a funny way to
> document changes (and was surprised when I told him a patch can be
> applied automatically, even if the original file is not exactly the same).
> But he is a typical windows-trained programmer. Lazy, unmotivated and
> happily married. He does programming by accident (was smart enough
> to learn the basics of the trade). Why would he want to take the an
> extra step of figuring out what that strange "0001-...patch" means?
> Now, I know him, would never think about sending him a real patch.
> I'm kinda grown and tired, and need the bastard, too. Someone younger
> will just call him idiot and "improve" the situation by telling him about
> "stupid windows" and "he should the right ways". Just to be answered
> "it worked for some millions programmers before you" and "I told your
> manager you making problems and are hard to communicate with".
> 
> People often understand "funny ways" the others may have. They
> don't like been told they are wrong or stupid (especially when they
> actually are stupid).


I still don't see the problem. When he understands (and uses) git, the 
name and look of the patch will become blindingly clear to him, and then 
it doesn't matter if it's called .txt or .patch. He might even have some 
tool by then that displays patches color-coded and what-not (there are a 
plethora of such tools for Windows already, most of which register 
.patch and .diff as file-types they handle).

Otoh, *until* he uses git, the change doesn't affect him, so why bother 
catering for his needs?

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Andreas Ericsson wrote:

> I still don't see the problem. When he understands (and uses) git, the 
> name and look of the patch will become blindingly clear to him,

Not so. Never underestimate mental inertia.

Recently, a co-worker (in a database-backed webapplication project!) asked 
what an SQL injection might be.

Of 5 developers, only 2 were able to answer!

Face it, there are a lot of incompetent programmers out there. And I am 
very happy you put up with me on this list.

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Andreas Ericsson <ae@op5.se> wrote:
> Alex Riesen wrote:
> > On 1/18/07, Josh Boyer <jwboyer@gmail.com> wrote:
> >> > As much as like Unix and everything related, I think it is
> >> > not reasonable to try to change the majority. Not unless
> >> > we have something earth-shattering. Well, git is, but
> >> > 0001-fix....patch in email attachment probably not.
> >>
> >> I would venture to say that the _majority_ of git users are not using
> >> Windows.
> >
> > The _real_ majority of the programmers desperately need a better
> > VCS than CVS, SVN, Perforce, SourceSafe, ClearCase, etc.
>
> They're free to use git ofcourse, provided they install cygwin or help
> migrate it to run natively on windows. I don't think anyone would cry if

My example had cygwin installed for him. Is it too unusual?

> a competent cross-platform programmer stepped up and started submitting
> patches to get git working on windows without having to resort to the
> cygwin emulation layer.

don't think so. I _would_ cry seeing how fork(2) gets ported to Windows,
and you will, probably... after seeing how it is done in cygwin.

> The thing is, no-one's getting paid for it, so until someone *does* step
> up, it won't happen, as 95% of the *current* git users are still running
> what we on this list will indefinitely refer to as "a sane OS".

95% of the current users probably is not even 1% of all programmers
who would gladly use it and maybe less than a fraction of percent of
the ones who need it.

> > People often understand "funny ways" the others may have. They
> > don't like been told they are wrong or stupid (especially when they
> > actually are stupid).
>
> I still don't see the problem. When he understands (and uses) git, the

he does not use git. He knows what the patch is (now I have explained
it to him), I suppose he would still be mildly surprised seeing a file.patch,
but he'll probably recover.

> name and look of the patch will become blindingly clear to him, and then
> it doesn't matter if it's called .txt or .patch. He might even have some
> tool by then that displays patches color-coded and what-not (there are a
> plethora of such tools for Windows already, most of which register
> .patch and .diff as file-types they handle).

That wont happen until windows registers it for them. And that, again,
wont ever happen. Every user will register .patch to notepad, visual
studio or ultraedit or something else for himself.

> Otoh, *until* he uses git, the change doesn't affect him,

actually, patch works for git patches, so format-patch produces
files useful output for anyone.

BTW, Junio, how about making the _default_ settable at compile time?
It'd be reasonable to allow local installations choose to default to what
they find the most paranoid?

> so why bother catering for his needs?

I don't know. It's kind of good style lately.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Alex Riesen wrote:

> [discussion about suffix being "patch" or "txt" per default]
>
> BTW, Junio, how about making the _default_ settable at compile time?
> It'd be reasonable to allow local installations choose to default to what
> they find the most paranoid?

Better control that with templates.

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > BTW, Junio, how about making the _default_ settable at compile time?
> > It'd be reasonable to allow local installations choose to default to what
> > they find the most paranoid?
>
> Better control that with templates.
>

Do we have a template for config already? I thought they were only for hooks...

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Alex Riesen wrote:

> On 1/18/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > BTW, Junio, how about making the _default_ settable at compile time?
> > > It'd be reasonable to allow local installations choose to default to what
> > > they find the most paranoid?
> > 
> > Better control that with templates.
> > 
> 
> Do we have a template for config already? I thought they were only for
> hooks...

The template mechanism can handle _all_ files in GIT_DIR. Just drop a 
"config" into the templates directory, and you're settled.

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> > > > BTW, Junio, how about making the _default_ settable at compile time?
> > > > It'd be reasonable to allow local installations choose to default to what
> > > > they find the most paranoid?
> > >
> > > Better control that with templates.
> > >
> >
> > Do we have a template for config already? I thought they were only for
> > hooks...
>
> The template mechanism can handle _all_ files in GIT_DIR. Just drop a
> "config" into the templates directory, and you're settled.

I'm settled! From now on I will never have any objections regarding
any defaults as long as they have a config option :)

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Josh Boyer]

On 1/18/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> > The template mechanism can handle _all_ files in GIT_DIR. Just drop a
> > "config" into the templates directory, and you're settled.
>
> I'm settled! From now on I will never have any objections regarding
> any defaults as long as they have a config option :)

Whew.  Cool :)

Junio, will this go into git at some point soon-ish?  I'm looking
forward to it...

josh

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Josh Boyer wrote:

> On 1/18/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> > > The template mechanism can handle _all_ files in GIT_DIR. Just drop a
> > > "config" into the templates directory, and you're settled.
> > 
> > I'm settled! From now on I will never have any objections regarding
> > any defaults as long as they have a config option :)
> 
> Whew.  Cool :)
> 
> Junio, will this go into git at some point soon-ish?  I'm looking
> forward to it...

You mean the templates mechanism? Has been in git since

	v0.99.4~43 (Tue Aug 2 16:45:21 2005 -0700)

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Josh Boyer]

On 1/18/07, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
> Hi,
>
> On Thu, 18 Jan 2007, Josh Boyer wrote:
>
> > On 1/18/07, Alex Riesen <raa.lkml@gmail.com> wrote:
> > > > The template mechanism can handle _all_ files in GIT_DIR. Just drop a
> > > > "config" into the templates directory, and you're settled.
> > >
> > > I'm settled! From now on I will never have any objections regarding
> > > any defaults as long as they have a config option :)
> >
> > Whew.  Cool :)
> >
> > Junio, will this go into git at some point soon-ish?  I'm looking
> > forward to it...
>
> You mean the templates mechanism? Has been in git since

No, I mean a switch to using ".patch" as the default and the option to
specify your own suffix.  The actual reason this thread was started :)

josh

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> don't think so. I _would_ cry seeing how fork(2) gets ported to Windows,
> and you will, probably... after seeing how it is done in cygwin.

AFAIK there's not a strong reason to keep fork() in Git.

Currently anytime we fork a process its to perform a small amount
of file descriptor redirection and then immediately exec some other
executable, or a hook script.  In other words we probably could
convert all current uses of fork to something like in run-command.c,
which a Windows port could then easily replace using CreateProcess().

But removing fork isn't worth doing until someone is seriously
trying to port Git onto Windows without Cygwin.  The current code
works on sane OSes and isn't broken, so why fix it?
 
-- 
Shawn.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > don't think so. I _would_ cry seeing how fork(2) gets ported to Windows,
> > and you will, probably... after seeing how it is done in cygwin.
>
> AFAIK there's not a strong reason to keep fork() in Git.
>
> Currently anytime we fork a process its to perform a small amount
> of file descriptor redirection and then immediately exec some other
> executable, or a hook script.  In other words we probably could
> convert all current uses of fork to something like in run-command.c,
> which a Windows port could then easily replace using CreateProcess().

I count 17 instances (excluding run_command). At least fetch-pack
is not trivial (the sideband code. Could be done in a  thread, which
is not portable just as well).

> But removing fork isn't worth doing until someone is seriously
> trying to port Git onto Windows without Cygwin.  The current code
> works on sane OSes and isn't broken, so why fix it?

Right.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Steven Grimm]

Alex Riesen wrote:
> I count 17 instances (excluding run_command). At least fetch-pack
> is not trivial (the sideband code. Could be done in a  thread, which
> is not portable just as well).

I looked at that briefly a while ago -- at the prompting of a Windows 
developer friend of mine who has some interest in git -- and it seemed 
like the best thing for portability to non-fork()ing systems would 
probably be a refactor. It looked to me like it'd be possible to 
reorganize the code such that it'd work all in one process with no 
threads or forking or anything. Not *trivial*, mind you, but possible. 
There's nothing in the code path that I saw (I didn't analyze it 
super-thoroughly) that looked like it actually needed to run in parallel.

IMO it's worth doing at some point post-1.5.0 simply because it means 
one less hurdle for someone who's looking to port Git to Windows. Plus 
it'll probably make the code slightly more efficient even on Linux and 
friends; there'd be less context-switching latency.

 From my brief look, that was the only nontrivial use of fork(). Almost 
all of the rest are simple fork/exec pairs.

Of course, the bigger hurdle for a native Windows port is all the shell 
scripts. Mercurial solves that by using Python for all its scripts, 
which at least has a native Windows version that can be installed. I 
wonder if git will/should eventually move its remaining shell scripts to 
Perl for that reason, Perl being git's de facto non-shell scripting 
language of choice.

-Steve

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Schindelin]

Hi,

On Thu, 18 Jan 2007, Steven Grimm wrote:

> I looked at that briefly a while ago -- at the prompting of a Windows 
> developer friend of mine who has some interest in git -- and it seemed 
> like the best thing for portability to non-fork()ing systems would 
> probably be a refactor. It looked to me like it'd be possible to 
> reorganize the code such that it'd work all in one process with no 
> threads or forking or anything. Not *trivial*, mind you, but possible. 
> There's nothing in the code path that I saw (I didn't analyze it 
> super-thoroughly) that looked like it actually needed to run in 
> parallel.

ssh transport comes to mind, as well as paging functionality. Sometimes we 
fork() to catch out-of-memory errors more gracefully.

> Of course, the bigger hurdle for a native Windows port is all the shell 
> scripts. Mercurial solves that by using Python for all its scripts, 
> which at least has a native Windows version that can be installed. I 
> wonder if git will/should eventually move its remaining shell scripts to 
> Perl for that reason, Perl being git's de facto non-shell scripting 
> language of choice.

Yeah, sure. We had no problems with Perl ;-)

Seriously, IMHO bash is a smaller dependency: here you can at least rely 
on which extensions are present (none), and which path-name munging is 
present on Windows (/c/windows).

No, the best is not to migrate shell scripts to Perl, but to C.

Ciao,
Dscho

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Johannes Sixt]

"Shawn O. Pearce" wrote:
> AFAIK there's not a strong reason to keep fork() in Git.
> 
> Currently anytime we fork a process its to perform a small amount
> of file descriptor redirection and then immediately exec some other
> executable, or a hook script.  In other words we probably could
> convert all current uses of fork to something like in run-command.c,
> which a Windows port could then easily replace using CreateProcess().
> 
> But removing fork isn't worth doing until someone is seriously
> trying to port Git onto Windows without Cygwin.  The current code
> works on sane OSes and isn't broken, so why fix it?

I'm doing just that (MinGW port).

I've come up with a function spawnvpe_pipe(), which hides all the scary
details of fork+exec with dup2's and close's. It could probably easily
be merged into run_command(), but I haven't tried that, yet.

I'll try to push out what I have to repo.or.cz over the weekend. 

-- Hannes

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Jakub Narebski]

Alex Riesen wrote:

> BTW, Junio, how about making the _default_ settable at compile time?
> It'd be reasonable to allow local installations choose to default to what
> they find the most paranoid?

You can always modify templates.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Shawn O. Pearce]

Alex Riesen <raa.lkml@gmail.com> wrote:
> The _real_ majority of the programmers desperately need a better
> VCS than CVS, SVN, Perforce, SourceSafe, ClearCase, etc.

Yes.  But...

Yesterday I had a conversation with the software configuration
management guy at my day-time-pays-the-bills organization.
They are seriously looking at Perforce and ClearCase, as these are
lightyears ahead of what we have already (PVCS Version Manager).
They also have 1-800-my-vendor telephone numbers which you can
call and scream at someone when the tool corrupts its internal
database[*1*], or when you cannot figure out what the "Checkout"
action in the context menu does[*2*].

However my fellow developers and I use Git.  We export our changes
out to PVCS Version Manager via an *ugly* Perl script that I would
never actually wish on anyone (which is one reason why its not
contributed as git-pvcsexport).  Configuration management guy won't
even look at Git's real strengths as it lacks the all-important
1-800-git-help[*3*] phone number.

Yesterday we spent 4 half-man-days (4 developers working together
all morning) trying to get a configuration of random files from
PVCS Version Manager which compiled.  In Git this would have been as
simple as merging the two topics that management wanted moved to the
next level of testing.  But since our organization doesn't actually
use Git and multiple topics affected the same files, well, yea, it
was a mess.  Unfortunately calling 1-800-my-vendor yields a "don't do
that" from our vendor and nothing more in support.  I'm very glad
we pay them for the privilege of having a 1-800-my-vendor phone
number in our rolodex.  Yesterday it cost us over $1600 in labor.

> Yes. For me and you. One of my coworkers knows nothing about patches,
> but wants (and perfectly able to) review my code. He has usable brains
> and is able to figure out what "+" and "-" is (he has, by now). He hasn't
> even realized that it was an automatically generated information, as
> I sent a patch to him first time, thought it was just a funny way to
> document changes (and was surprised when I told him a patch can be
> applied automatically, even if the original file is not exactly the same).
> But he is a typical windows-trained programmer.

I work with a few people who would rather copy and paste their
changed parts into an email, then color them with pretty colors
like red, green, blue, black, orange in Outlook (and all in the
same email too).  These then get sent to me for code review.

Prior to us using Git it may make some sense, as we did not have
a diff tool available.  Now that everyone is using Git and working
in isolated topic branches (and doing very well with that concept)
I still get these emails.  People seem to have an easy time grasping
the idea that Git is tracking their changes and when combined with
my git-pvcsexport (see above) it just Does The Right Thing(tm)
later on.  But they have a hard time grasping the idea that Git can
export these as a diff, or that they could just push their topic
branch to me so I can pop it open in gitk.  *sigh*


[*1*] This has happened to me with Perforce more than once.  Not a
      happy thought.  Never with Git.

[*2*] Yes, really, some of our version control users have difficulty
      grasping a concept like "checkout".  They *definately* have
      an issue with Git's "checkout -b" concept.

[*3*] Already taken.  Oddly enough by a company that could almost
      be considered to be a competiter to my day-time-pays-the-bills
      organization.
 
-- 
Shawn.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alex Riesen]

On 1/18/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> > The _real_ majority of the programmers desperately need a better
> > VCS than CVS, SVN, Perforce, SourceSafe, ClearCase, etc.
>
> Yes.  But...
>
> Yesterday I had a conversation with the software configuration
> management guy at my day-time-pays-the-bills organization.
> They are seriously looking at Perforce and ClearCase, as these are
> lightyears ahead of what we have already (PVCS Version Manager).
> They also have 1-800-my-vendor telephone numbers which you can
> call and scream at someone when the tool corrupts its internal
> database[*1*], or when you cannot figure out what the "Checkout"
> action in the context menu does[*2*].
>
> However my fellow developers and I use Git.  We export our changes
> out to PVCS Version Manager via an *ugly* Perl script that I would
> never actually wish on anyone (which is one reason why its not
> contributed as git-pvcsexport).  Configuration management guy won't
> even look at Git's real strengths as it lacks the all-important
> 1-800-git-help[*3*] phone number.

Of course, he is the who'll do the yelling. It's you who'll need support.
Standard Perforce replies: "it is not supported" (if you need branches
as in Git), or "we will probably look into it" (if you point them to a bug.
They never really do). Worst performance in the industry, too.

ClearCase had a sleep(6 /*sec*/) before writing a file in local checkouts
on linux, for whatever reason :) It at least knows about oriented graphs.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Andreas Ericsson]

Shawn O. Pearce wrote:
> Alex Riesen <raa.lkml@gmail.com> wrote:
> 
> However my fellow developers and I use Git.  We export our changes
> out to PVCS Version Manager via an *ugly* Perl script that I would
> never actually wish on anyone (which is one reason why its not
> contributed as git-pvcsexport).  Configuration management guy won't
> even look at Git's real strengths as it lacks the all-important
> 1-800-git-help[*3*] phone number.
> 

Would a +46-31 number work? If so, you could give me a call when you're
having trouble. I'd probably end up asking the list, or bribing Junio
with beer to answer for me, but the fee would be low (how much is a
dozen beers in Japan?), so perhaps it'd be worth it ;-)

On a serious note, it's probably about time the world saw its first
commercial git support company. It's legal to package and sell GPL'd
code. Many companies have already proven that it can be a very
lucrative business.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Shawn O. Pearce]

Andreas Ericsson <ae@op5.se> wrote:
> Would a +46-31 number work? If so, you could give me a call when you're
> having trouble. I'd probably end up asking the list, or bribing Junio
> with beer to answer for me, but the fee would be low (how much is a
> dozen beers in Japan?), so perhaps it'd be worth it ;-)

Heh.  The thing is, with me "on staff" I doubt you can get much
better support.  I know Git very well, almost as well as Linus,
Junio, Nico, and Johannes (sorry, no particular order there).
What I don't know off the top of my head, I know where the source
code for it is, and I can read and understand it rather quickly.

When something breaks, I can usually fix it myself, and that usually
results in a patch to Junio just hours after I discover the problem.
Most of the time the patch is worthy of inclusion and Junio picks
it up.  You can't get that kind of response from a commerical vendor,
at least not without forking over bucket loads of cash first.

The problem is, the organization has strict rules about recommending
yourself as a vendor.  But recommending a guy half way around the
world who works for beer is probably in compliance.  :-)
 
> On a serious note, it's probably about time the world saw its first
> commercial git support company. It's legal to package and sell GPL'd
> code. Many companies have already proven that it can be a very
> lucrative business.

I've thought about doing this myself.  I'm just so short on time
that developing a business providing support would probably push me
way over the edge.  Ideally I'd love to have such a venture make its
money off support contracts and a small markup on dead-tree forms of
open-source Git documentation.  I'd also love to see such a venture
be able to support a Git developer or two full-time, making sure that
all of their work is getting folded back into the main git.git tree.
Which of course implies they can't be heading off in directions
that the rest of the group finds useless/pointless/stupid/etc.

Wishful thinking.  Back to reality.

-- 
Shawn.

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Andreas Ericsson]

Shawn O. Pearce wrote:
> Andreas Ericsson <ae@op5.se> wrote:
>> Would a +46-31 number work? If so, you could give me a call when you're
>> having trouble. I'd probably end up asking the list, or bribing Junio
>> with beer to answer for me, but the fee would be low (how much is a
>> dozen beers in Japan?), so perhaps it'd be worth it ;-)
> 
> Heh.  The thing is, with me "on staff" I doubt you can get much
> better support.  I know Git very well, almost as well as Linus,
> Junio, Nico, and Johannes (sorry, no particular order there).
> What I don't know off the top of my head, I know where the source
> code for it is, and I can read and understand it rather quickly.
> 

Ya, I knows. :)

> When something breaks, I can usually fix it myself, and that usually
> results in a patch to Junio just hours after I discover the problem.
> Most of the time the patch is worthy of inclusion and Junio picks
> it up.  You can't get that kind of response from a commerical vendor,
> at least not without forking over bucket loads of cash first.
> 
> The problem is, the organization has strict rules about recommending
> yourself as a vendor.  But recommending a guy half way around the
> world who works for beer is probably in compliance.  :-)
>  

That was my devilishly clever plan; To provide support to someone who
knows the thing I'm supposed to support a lot better than myself, while
getting some free beer in the process ;-)

>> On a serious note, it's probably about time the world saw its first
>> commercial git support company. It's legal to package and sell GPL'd
>> code. Many companies have already proven that it can be a very
>> lucrative business.
> 
> I've thought about doing this myself.  I'm just so short on time
> that developing a business providing support would probably push me
> way over the edge.  Ideally I'd love to have such a venture make its
> money off support contracts and a small markup on dead-tree forms of
> open-source Git documentation.  I'd also love to see such a venture
> be able to support a Git developer or two full-time, making sure that
> all of their work is getting folded back into the main git.git tree.
> Which of course implies they can't be heading off in directions
> that the rest of the group finds useless/pointless/stupid/etc.
> 
> Wishful thinking.  Back to reality.
> 

This is a case where "Think Big" isn't enough, and you need to "Think
Bigger". Don't settle for shipping help-docs on git and answering the
phone. Sell pre-packaged versions of git, with a pre-installed Linux
server with raided disks and a nifty backup-solution (just sell a second
server and use an update-hook to replicate everything to that one, then
you're done). You could easily charge up to $2,500 / user for providing
"A fully integrated VCS / backup solution, with full failover to ensure
100% efficiency in your day-to-day job". Tack on another 10k for the
two servers and another 1k / dev to go to a training seminar and you're
good to go.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Martin Langhoff]

On 1/19/07, Shawn O. Pearce <spearce@spearce.org> wrote:
> Wishful thinking.  Back to reality.

Not necessarily ;-) but I'm not sure if the time is right for an
independend services company doing _only_ git.

However, git is the kind of SCM that a big distro needs to keep track
of all their "vendor branches" or "patches against upstream". Ubuntu
pays at least one full-time SCM developer, Martin Pool, to maintain
Bazaar NG and accesory tools.

IIRC MySQL was looking quite seriously to drop BK and with the savings
in licenses, can surely affort to hire a GIT guru to "train the
trainer", solve problems/bugs and write internal support tools. Others
will follow. It shouldn't be too hard to find the right place.

Or a large FOSS services focused company (like Catalyst) that uses git
and offers support as part of a larger bundle. (<spam>we're hiring,
and putting git hackery in your cv is a winner</spam>). There are
plenty of gaps and places for git hackers.

cheers


martin

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Martin Langhoff]

On 1/19/07, Andreas Ericsson <ae@op5.se> wrote:
> On a serious note, it's probably about time the world saw its first
> commercial git support company. It's legal to package and sell GPL'd
> code. Many companies have already proven that it can be a very
> lucrative business.

<spam, but on topic >
At Catalyst (~70 perl/java/php/c/c++ developers, mostly foss
development) we have switched internally to git for 80% of our active
projects and we do offer commercial support for a couple of clients.
It's not the only thing we do for those clients, but if an
organisation needed specifically support for git, we can help.

We aren't pushing/packaging/selling it because it doesn't work well
that way (for us at least). When it's about new tools, we prefer to
work with people who already know what they want ;-)

cheers,


martin

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Andreas Ericsson]

Junio C Hamano wrote:
> 
> Is it common for popular MUAs to have a single command that lets
> you specify a file and depending on its suffix paste it inline
> or make it an attachment?  I had an impression that most have
> separate commands for "read text from file (as opposed to
> typing)" and "attach a file (of random type, not necessarily and
> more often than not text)".
> 

Most have only "attach" through various means of point-and-click and 
drag-and-drop. Thunderbird too lacks the very basic ability of "include 
this file as part of the message". I still haven't been able to find an 
addon for it that does just that.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Lukas Sandström]

Andreas Ericsson wrote:
> Most have only "attach" through various means of point-and-click and
> drag-and-drop. Thunderbird too lacks the very basic ability of "include
> this file as part of the message". I still haven't been able to find an
> addon for it that does just that.

I use the External Editor extension with Thunderbird (it's mentioned in SubmittingPatches).

The script below is used as the external editor, which gives me an easy way to send patches
inline with Thunderbird.

/Lukas

#!/bin/bash

CONFFILE=~/.appprc

if [ -e "$CONFFILE" ] ; then
	LAST_DIR=`grep "^LAST_DIR=" $CONFFILE|sed -e 's/^LAST_DIR=//'`
	cd $LAST_DIR
else
	cd > /dev/null
fi

PATCH=$(zenity --file-selection)

if [ "$?" != "0" ] ; then
	#zenity --error --text "No patchfile given."
	exit 1
fi

cd - > /dev/null

SUBJECT=`sed -n -e '/^Subject: /p' $PATCH`
HEADERS=`sed -e '/^Subject: /d' $1`

echo "$SUBJECT" > $1
echo "$HEADERS" >> $1
sed -e '1,/^$/d' $PATCH >> $1

LAST_DIR=`dirname $PATCH`
sed -e 's@^LAST_DIR=.*@LAST_DIR='$LAST_DIR'@' $CONFFILE > $CONFFILE"_"
mv $CONFFILE"_" $CONFFILE

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Brian Gernhardt]

On Jan 18, 2007, at 7:40 AM, Andreas Ericsson wrote:

> Junio C Hamano wrote:
>> Is it common for popular MUAs to have a single command that lets
>> you specify a file and depending on its suffix paste it inline
>> or make it an attachment?  I had an impression that most have
>> separate commands for "read text from file (as opposed to
>> typing)" and "attach a file (of random type, not necessarily and
>> more often than not text)".
>
> Most have only "attach" through various means of point-and-click  
> and drag-and-drop. Thunderbird too lacks the very basic ability of  
> "include this file as part of the message". I still haven't been  
> able to find an addon for it that does just that.

The "easiest" way is to open it in an editor and Copy'n'Paste it into  
the message.  Unfortunately, not all clipboards or applications can  
be trusted not to mangle whitespace.  I learned that the hard way  
trying to use Mail.app to send out patches.  I ended up just  
installing Mutt.  Perhaps I should figure out how this "git-send- 
email" works, although it doesn't appear to support authentication...

~~ Brian

Re: [PATCH/POLL] git-format-patch: the default suffix is now .patch, not .txt

[Alexandre Julliard]

"Alex Riesen" <raa.lkml@gmail.com> writes:

> Also, how many mail clients know that .patch is actually
> a text and not application/binary? It'll make patch
> reviewing harder for some (not sure if I'd like a review
> of such a person, though).

OTOH such mail clients usually also think they can freely reformat
text files, leading to unusable patches.  A .patch extension would
help avoid that type of breakage, so I think it's a good idea.

-- 
Alexandre Julliard
julliard@winehq.org