[PATCH 0/2] Add commit message options for rebase --autosquash

[PATCH 0/2] Add commit message options for rebase --autosquash

[Pat Notz]

This change adds new command line options to git-commit to make it
easy to specify messages for commits destined for
'rebase -i --autosquash'.

The first commit adds the new options to git-commit.

The second commit adds tests of the new options.  The tests only look
for properly formed commit messages and expected error conditions.  They
do not test the rebase --autosquash functionality.

Pat Notz (2):
  commit: add message options for rebase --autosquash
  t7500: add tests of commit --fixup/--squash

 Documentation/git-commit.txt |   18 ++++++++++++++----
 builtin/commit.c             |   37 +++++++++++++++++++++++++++++++++----
 t/t7500-commit.sh            |   39 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 8 deletions(-)

-- 
1.7.2.3

[PATCH 1/2] commit: add message options for rebase --autosquash

[Pat Notz]

These options make it convenient to construct commit messages for use
with 'rebase --autosquash'.  The resulting commit message will be
"fixup! ..." or "squash! ..." where "..." is the subject line of the
specified commit message.

Example usage:
  $ git commit --fixup HEAD~2
  $ git commit --squash HEAD~5

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 Documentation/git-commit.txt |   18 ++++++++++++++----
 builtin/commit.c             |   37 +++++++++++++++++++++++++++++++++----
 2 files changed, 47 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 42fb1f5..1d1e0c8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,10 +9,10 @@ SYNOPSIS
 --------
 [verse]
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
-	   [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
-	   [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
-	   [--date=<date>] [--cleanup=<mode>] [--status | --no-status] [--]
-	   [[-i | -o ]<file>...]
+	   [(-c | -C | --fixup | --squash ) <commit>] [-F <file> | -m <msg>]
+	   [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
+	   [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
+	   [--status | --no-status] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -70,6 +70,16 @@ OPTIONS
 	Like '-C', but with '-c' the editor is invoked, so that
 	the user can further edit the commit message.
 
+--fixup=<commit>::
+	Construct a commit message for use with `rebase --autosquash`.
+	The commit message will be the subject line from the specified
+	commit with a prefix of "fixup! ".
+
+--squash=<commit>::
+	Construct a commit message for use with `rebase --autosquash`.
+	The commit message will be the subject line from the specified
+	commit with a prefix of "squash! ".
+
 --reset-author::
 	When used with -C/-c/--amend options, declare that the
 	authorship of the resulting commit now belongs of the committer.
diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..e525e78 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -57,6 +57,7 @@ static const char empty_amend_advice[] =
 static unsigned char head_sha1[20];
 
 static char *use_message_buffer;
+static char *fixup_message_buffer;
 static const char commit_editmsg[] = "COMMIT_EDITMSG";
 static struct lock_file index_lock; /* real index */
 static struct lock_file false_lock; /* used only for partial commits */
@@ -69,6 +70,7 @@ static enum {
 static const char *logfile, *force_author;
 static const char *template_file;
 static char *edit_message, *use_message;
+static char *fixup_message, *squash_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
@@ -124,6 +126,8 @@ static struct option builtin_commit_options[] = {
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+	OPT_STRING(0, "fixup", &fixup_message, "COMMIT", "use autosquash formatted message to fixup specified commit"),
+	OPT_STRING(0, "squash", &squash_message, "COMMIT", "use autosquash formatted message to squash specified commit"),
 	OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
@@ -586,6 +590,10 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
 		hook_arg1 = "commit";
 		hook_arg2 = use_message;
+	} else if (fixup_message || squash_message) {
+		strbuf_addstr(&sb, fixup_message_buffer);
+		free(fixup_message_buffer);
+		hook_arg1 = "message";
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die_errno("could not read MERGE_MSG");
@@ -863,7 +871,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (force_author && renew_authorship)
 		die("Using both --reset-author and --author does not make sense");
 
-	if (logfile || message.len || use_message)
+	if (logfile || message.len || use_message || fixup_message || squash_message)
 		use_editor = 0;
 	if (edit_flag)
 		use_editor = 1;
@@ -883,15 +891,19 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		f++;
 	if (edit_message)
 		f++;
+	if (fixup_message)
+		f++;
+	if (squash_message)
+		f++;
 	if (logfile)
 		f++;
 	if (f > 1)
-		die("Only one of -c/-C/-F can be used.");
+		die("Only one of -c/-C/-F/--fixup/--squash can be used.");
 	if (message.len && f > 0)
-		die("Option -m cannot be combined with -c/-C/-F.");
+		die("Option -m cannot be combined with -c/-C/-F/--fixup/--squash.");
 	if (edit_message)
 		use_message = edit_message;
-	if (amend && !use_message)
+	if (amend && (!use_message && !fixup_message && !squash_message))
 		use_message = "HEAD";
 	if (!use_message && renew_authorship)
 		die("--reset-author can be used only with -C, -c or --amend.");
@@ -932,6 +944,23 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		if (enc != utf8)
 			free(enc);
 	}
+	if (fixup_message || squash_message) {
+		unsigned char sha1[20];
+		struct commit *commit;
+		const char * target_message = fixup_message ? fixup_message : squash_message;
+		const char * msg_fmt = fixup_message ? "fixup! %s" : "squash! %s";
+		struct strbuf buf = STRBUF_INIT;
+		struct pretty_print_context ctx = {0};
+
+		if (get_sha1(target_message, sha1))
+			die("could not lookup commit %s", target_message);
+		commit = lookup_commit_reference(sha1);
+		if (!commit || parse_commit(commit))
+			die("could not parse commit %s", target_message);
+
+		format_commit_message(commit, msg_fmt, &buf, &ctx);
+		fixup_message_buffer = strbuf_detach(&buf, NULL);
+	}
 
 	if (!!also + !!only + !!all + !!interactive > 1)
 		die("Only one of --include/--only/--all/--interactive can be used.");
-- 
1.7.2.3

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Stephen Boyd]

On 09/16/2010 06:39 PM, Pat Notz wrote:
> These options make it convenient to construct commit messages for use
> with 'rebase --autosquash'.  The resulting commit message will be
> "fixup! ..." or "squash! ..." where "..." is the subject line of the
> specified commit message.
> 
> Example usage:
>   $ git commit --fixup HEAD~2
>   $ git commit --squash HEAD~5
> 
> Signed-off-by: Pat Notz <patnotz@gmail.com>
> ---

So far I've been using an alias for these, but I suppose making them
real features of git could be worthwhile. What are the benefits with
this approach vs. an alias?

> @@ -863,7 +871,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  	if (force_author && renew_authorship)
>  		die("Using both --reset-author and --author does not make sense");
>  
> -	if (logfile || message.len || use_message)
> +	if (logfile || message.len || use_message || fixup_message || squash_message)
>  		use_editor = 0;
>  	if (edit_flag)
>  		use_editor = 1;

The whole point of squash is to combine two commit texts, right?
Otherwise wouldn't you use --fixup where you throw away the text
eventually and thus don't want to open an editor?

> @@ -883,15 +891,19 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		f++;
>  	if (edit_message)
>  		f++;
> +	if (fixup_message)
> +		f++;
> +	if (squash_message)
> +		f++;
>  	if (logfile)
>  		f++;
>  	if (f > 1)
> -		die("Only one of -c/-C/-F can be used.");
> +		die("Only one of -c/-C/-F/--fixup/--squash can be used.");
>  	if (message.len && f > 0)
> -		die("Option -m cannot be combined with -c/-C/-F.");
> +		die("Option -m cannot be combined with -c/-C/-F/--fixup/--squash.");


Furthering that point, perhaps I want to squash this commit into another
commit using the commit text from yet another commit or just with an
extra note from the command line (-m). Perhaps this is where the benefit
over an alias comes in?

>  	if (edit_message)
>  		use_message = edit_message;
> -	if (amend && !use_message)
> +	if (amend && (!use_message && !fixup_message && !squash_message))
>  		use_message = "HEAD";
>  	if (!use_message && renew_authorship)
>  		die("--reset-author can be used only with -C, -c or --amend.");
> @@ -932,6 +944,23 @@ static int parse_and_validate_options(int argc, const char *argv[],
>  		if (enc != utf8)
>  			free(enc);
>  	}
> +	if (fixup_message || squash_message) {
> +		unsigned char sha1[20];
> +		struct commit *commit;
> +		const char * target_message = fixup_message ? fixup_message : squash_message;
> +		const char * msg_fmt = fixup_message ? "fixup! %s" : "squash! %s";

Style nit: stick the * to the variable.

I read this and became confused. fixup_message? target_message? Perhaps
it should be renamed to fixup_commit, squash_commit, target_commit?

> +		struct strbuf buf = STRBUF_INIT;
> +		struct pretty_print_context ctx = {0};
> +
> +		if (get_sha1(target_message, sha1))
> +			die("could not lookup commit %s", target_message);
> +		commit = lookup_commit_reference(sha1);
> +		if (!commit || parse_commit(commit))
> +			die("could not parse commit %s", target_message);
> +
> +		format_commit_message(commit, msg_fmt, &buf, &ctx);
> +		fixup_message_buffer = strbuf_detach(&buf, NULL);
> +	}
>  

Is it necessary to do this block of code here? Couldn't you lookup and
format the commit in prepare_to_commit()? Then we wouldn't have to
allocate another strbuf and the "message" code would be more centralized.

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Pat Notz]

On Fri, Sep 17, 2010 at 2:36 AM, Stephen Boyd <bebarino@gmail.com> wrote:
> On 09/16/2010 06:39 PM, Pat Notz wrote:
>> These options make it convenient to construct commit messages for use
>> with 'rebase --autosquash'.  The resulting commit message will be
>> "fixup! ..." or "squash! ..." where "..." is the subject line of the
>> specified commit message.
>>
>> Example usage:
>>   $ git commit --fixup HEAD~2
>>   $ git commit --squash HEAD~5
>>
>> Signed-off-by: Pat Notz <patnotz@gmail.com>
>> ---
>
> So far I've been using an alias for these, but I suppose making them
> real features of git could be worthwhile. What are the benefits with
> this approach vs. an alias?

Mainly it's convenience.  The rebase --autosquash feature seems too
hard to use without this or an alias and making everyone code their
own alias seems a lot to ask.

Still, I admit that I was concerned with adding yet another option to
git-commit.  If enough people object, I can live with that.

>> @@ -863,7 +871,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
>>       if (force_author && renew_authorship)
>>               die("Using both --reset-author and --author does not make sense");
>>
>> -     if (logfile || message.len || use_message)
>> +     if (logfile || message.len || use_message || fixup_message || squash_message)
>>               use_editor = 0;
>>       if (edit_flag)
>>               use_editor = 1;
>
> The whole point of squash is to combine two commit texts, right?
> Otherwise wouldn't you use --fixup where you throw away the text
> eventually and thus don't want to open an editor?

Good point.  Admittedly, I was focusing on the 'fixup' case but squash
needs to open the editor with the first line pre-filled.

>
>> @@ -883,15 +891,19 @@ static int parse_and_validate_options(int argc, const char *argv[],
>>               f++;
>>       if (edit_message)
>>               f++;
>> +     if (fixup_message)
>> +             f++;
>> +     if (squash_message)
>> +             f++;
>>       if (logfile)
>>               f++;
>>       if (f > 1)
>> -             die("Only one of -c/-C/-F can be used.");
>> +             die("Only one of -c/-C/-F/--fixup/--squash can be used.");
>>       if (message.len && f > 0)
>> -             die("Option -m cannot be combined with -c/-C/-F.");
>> +             die("Option -m cannot be combined with -c/-C/-F/--fixup/--squash.");
>
>
> Furthering that point, perhaps I want to squash this commit into another
> commit using the commit text from yet another commit or just with an
> extra note from the command line (-m). Perhaps this is where the benefit
> over an alias comes in?

That's a good use-case.  I'll re-work the --squash option.

>
>>       if (edit_message)
>>               use_message = edit_message;
>> -     if (amend && !use_message)
>> +     if (amend && (!use_message && !fixup_message && !squash_message))
>>               use_message = "HEAD";
>>       if (!use_message && renew_authorship)
>>               die("--reset-author can be used only with -C, -c or --amend.");
>> @@ -932,6 +944,23 @@ static int parse_and_validate_options(int argc, const char *argv[],
>>               if (enc != utf8)
>>                       free(enc);
>>       }
>> +     if (fixup_message || squash_message) {
>> +             unsigned char sha1[20];
>> +             struct commit *commit;
>> +             const char * target_message = fixup_message ? fixup_message : squash_message;
>> +             const char * msg_fmt = fixup_message ? "fixup! %s" : "squash! %s";
>
> Style nit: stick the * to the variable.
>

Oops, thanks.

> I read this and became confused. fixup_message? target_message? Perhaps
> it should be renamed to fixup_commit, squash_commit, target_commit?
>

I was mostly trying to reduce duplicate code for the two cases... but,
I bet when I re-work --squash this will go away.

>> +             struct strbuf buf = STRBUF_INIT;
>> +             struct pretty_print_context ctx = {0};
>> +
>> +             if (get_sha1(target_message, sha1))
>> +                     die("could not lookup commit %s", target_message);
>> +             commit = lookup_commit_reference(sha1);
>> +             if (!commit || parse_commit(commit))
>> +                     die("could not parse commit %s", target_message);
>> +
>> +             format_commit_message(commit, msg_fmt, &buf, &ctx);
>> +             fixup_message_buffer = strbuf_detach(&buf, NULL);
>> +     }
>>
>
> Is it necessary to do this block of code here? Couldn't you lookup and
> format the commit in prepare_to_commit()? Then we wouldn't have to
> allocate another strbuf and the "message" code would be more centralized.
>

Probably not, I was mostly trying to follow the example from the
use_message (-C/-c) feature.  It *would* be nice to avoid the extra
memory (de)alloc.

Thanks for the great feedback!

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Bryan Drewery]

Stephen Boyd wrote:
> On 09/16/2010 06:39 PM, Pat Notz wrote:
>   
>> These options make it convenient to construct commit messages for use
>> with 'rebase --autosquash'.  The resulting commit message will be
>> "fixup! ..." or "squash! ..." where "..." is the subject line of the
>> specified commit message.
>>
>> Example usage:
>>   $ git commit --fixup HEAD~2
>>   $ git commit --squash HEAD~5
>>
>> Signed-off-by: Pat Notz <patnotz@gmail.com>
>> ---
>>     
>
> So far I've been using an alias for these, but I suppose making them
> real features of git could be worthwhile. What are the benefits with
> this approach vs. an alias?
>
>   

I keep wanting to do these at commit time.

What are the alternative aliases?

Bryan

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Stephen Boyd]

On Fri, Sep 17, 2010 at 9:14 AM, Bryan Drewery <bryan@shatow.net> wrote:
> Stephen Boyd wrote:
>>
>> On 09/16/2010 06:39 PM, Pat Notz wrote:
>>
>>>
>>> These options make it convenient to construct commit messages for use
>>> with 'rebase --autosquash'.  The resulting commit message will be
>>> "fixup! ..." or "squash! ..." where "..." is the subject line of the
>>> specified commit message.
>>>
>>> Example usage:
>>>  $ git commit --fixup HEAD~2
>>>  $ git commit --squash HEAD~5
>>>
>>> Signed-off-by: Pat Notz <patnotz@gmail.com>
>>> ---
>>>
>>
>> So far I've been using an alias for these, but I suppose making them
>> real features of git could be worthwhile. What are the benefits with
>> this approach vs. an alias?
>>
>>
>
> I keep wanting to do these at commit time.
>
> What are the alternative aliases?
>

`git fixup' is aliased to `!f() { git commit -m "$(git show -s
--pretty='format:fixup! %s%n%nFixup for %h%n' "$1")" $2; }; f'
`git squash' is aliased to `!f() { git commit -m "$(git show -s
--pretty='format:squash! %s%n%n' "$1")" -e $2; }; f'

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Bryan Drewery]

>>>> These options make it convenient to construct commit messages for use
>>>> with 'rebase --autosquash'.  The resulting commit message will be
>>>> "fixup! ..." or "squash! ..." where "..." is the subject line of the
>>>> specified commit message.
>>>>         
> `git fixup' is aliased to `!f() { git commit -m "$(git show -s
> --pretty='format:fixup! %s%n%nFixup for %h%n' "$1")" $2; }; f'
> `git squash' is aliased to `!f() { git commit -m "$(git show -s
> --pretty='format:squash! %s%n%n' "$1")" -e $2; }; f'
>   
Thanks.

I had missed that this was for the commit message. Thought it was doing 
the rebase as well.

Bryan

Re: [PATCH 1/2] commit: add message options for rebase --autosquash

[Junio C Hamano]

Stephen Boyd <bebarino@gmail.com> writes:

> The whole point of squash is to combine two commit texts, right?
> Otherwise wouldn't you use --fixup where you throw away the text
> eventually and thus don't want to open an editor?

You mentioned almost everything I wanted to say.  I think --fixup makes
sense, but I doubt --squash does, _unless_ its interactactions with other
message pre-filling options, e.g. -m, -F, -c, are really well thought
out.

[PATCH 2/2] t7500: add tests of commit --fixup/--squash

[Pat Notz]

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 t/t7500-commit.sh |   39 +++++++++++++++++++++++++++++++++++++++
 1 files changed, 39 insertions(+), 0 deletions(-)

diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index aa9c577..cd21f67 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -215,4 +215,43 @@ test_expect_success 'Commit a message with --allow-empty-message' '
 	commit_msg_is "hello there"
 '
 
+commit_for_rebase_autosquash_setup() {
+	echo "first content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo &&
+	cat >log <<EOF
+target message subject line
+
+target message body line 1
+target message body line 2
+EOF
+	git commit -F log &&
+	echo "second content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo &&
+	git commit -m "intermediate commit" &&
+	echo "third content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo
+}
+
+test_expect_success 'commit --fixup provides correct one-line commit message' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --fixup HEAD~1 &&
+	commit_msg_is "fixup! target message subject line"
+'
+test_expect_success 'commit --squash provides correct one-line commit message' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --squash HEAD~1 &&
+	commit_msg_is "squash! target message subject line"
+'
+
+test_expect_success 'multiple commit message options must fail' '
+	echo changes >>foo &&
+	echo "message" >log &&
+	git add foo &&
+	test_must_fail git commit --fixup HEAD~1 --squash HEAD~1 &&
+	test_must_fail git commit --fixup HEAD~1 -m "cmdline message" &&
+	test_must_fail git commit --fixup HEAD~1 -F log &&
+	test_must_fail git commit --squash HEAD~1 -C HEAD~2 &&
+	test_must_fail git commit --squash HEAD~1 -c HEAD~2
+'
+
 test_done
-- 
1.7.2.3

[PATCHv2 0/4] Add commit message options for rebase --autosquash

[Pat Notz]

This patch series adds new command line options to git-commit to make
it easy to specify messages for commits correctly formatted for use
wit 'rebase -i --autosquash'.

This 2nd iteration address concerns raised earlier:
http://thread.gmane.org/gmane.comp.version-control.git/156369 .  Most
notably, --squash=COMMIT now works with -m/-c/-C/-F and uses the
editor when appropriate.

Pat Notz (4):
  commit: --fixup option for use with rebase --autosquash
  t7500: add tests of commit --fixup
  commit: --squash option for use with rebase --autosquash
  t7500: add tests of commit --squash

 Documentation/git-commit.txt |   19 +++++++--
 builtin/commit.c             |   58 ++++++++++++++++++++++++--
 t/t7500-commit.sh            |   90 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 158 insertions(+), 9 deletions(-)

-- 
1.7.3

[PATCHv2 1/4] commit: --fixup option for use with rebase --autosquash

[Pat Notz]

This option makes it convenient to construct commit messages for use
with 'rebase --autosquash'.  The resulting commit message will be
"fixup! ..." where "..." is the subject line of the specified commit
message.

Example usage:
  $ git commit --fixup HEAD~2

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 Documentation/git-commit.txt |   13 +++++++++----
 builtin/commit.c             |   23 +++++++++++++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 42fb1f5..3367f8f 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,10 +9,10 @@ SYNOPSIS
 --------
 [verse]
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
-	   [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
-	   [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
-	   [--date=<date>] [--cleanup=<mode>] [--status | --no-status] [--]
-	   [[-i | -o ]<file>...]
+	   [(-c | -C | --fixup) <commit>] [-F <file> | -m <msg>]
+	   [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
+	   [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
+	   [--status | --no-status] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -70,6 +70,11 @@ OPTIONS
 	Like '-C', but with '-c' the editor is invoked, so that
 	the user can further edit the commit message.
 
+--fixup=<commit>::
+	Construct a commit message for use with `rebase --autosquash`.
+	The commit message will be the subject line from the specified
+	commit with a prefix of "fixup! ".
+
 --reset-author::
 	When used with -C/-c/--amend options, declare that the
 	authorship of the resulting commit now belongs of the committer.
diff --git a/builtin/commit.c b/builtin/commit.c
index 66fdd22..0901616 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -69,6 +69,7 @@ static enum {
 static const char *logfile, *force_author;
 static const char *template_file;
 static char *edit_message, *use_message;
+static char *fixup_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
@@ -124,6 +125,7 @@ static struct option builtin_commit_options[] = {
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
+	OPT_STRING(0, "fixup", &fixup_message, "COMMIT", "use autosquash formatted message to fixup specified commit"),
 	OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
@@ -586,6 +588,17 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 		strbuf_add(&sb, buffer + 2, strlen(buffer + 2));
 		hook_arg1 = "commit";
 		hook_arg2 = use_message;
+	} else if (fixup_message) {
+		unsigned char sha1[20];
+		struct commit *commit;
+		struct pretty_print_context ctx = {0};
+		if (get_sha1(fixup_message, sha1))
+			die("could not lookup commit %s", fixup_message);
+		commit = lookup_commit_reference(sha1);
+		if (!commit || parse_commit(commit))
+			die("could not parse commit %s", fixup_message);
+		format_commit_message(commit, "fixup! %s\n\n", &sb, &ctx);
+		hook_arg1 = "message";
 	} else if (!stat(git_path("MERGE_MSG"), &statbuf)) {
 		if (strbuf_read_file(&sb, git_path("MERGE_MSG"), 0) < 0)
 			die_errno("could not read MERGE_MSG");
@@ -863,7 +876,7 @@ static int parse_and_validate_options(int argc, const char *argv[],
 	if (force_author && renew_authorship)
 		die("Using both --reset-author and --author does not make sense");
 
-	if (logfile || message.len || use_message)
+	if (logfile || message.len || use_message || fixup_message)
 		use_editor = 0;
 	if (edit_flag)
 		use_editor = 1;
@@ -883,15 +896,17 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		f++;
 	if (edit_message)
 		f++;
+	if (fixup_message)
+		f++;
 	if (logfile)
 		f++;
 	if (f > 1)
-		die("Only one of -c/-C/-F can be used.");
+		die("Only one of -c/-C/-F/--fixup can be used.");
 	if (message.len && f > 0)
-		die("Option -m cannot be combined with -c/-C/-F.");
+		die("Option -m cannot be combined with -c/-C/-F/--fixup.");
 	if (edit_message)
 		use_message = edit_message;
-	if (amend && !use_message)
+	if (amend && !use_message && !fixup_message)
 		use_message = "HEAD";
 	if (!use_message && renew_authorship)
 		die("--reset-author can be used only with -C, -c or --amend.");
-- 
1.7.3

Re: [PATCHv2 1/4] commit: --fixup option for use with rebase --autosquash

[Sverre Rabbelier]

Heya,

On Tue, Sep 21, 2010 at 22:25, Pat Notz <patnotz@gmail.com> wrote:
> This option makes it convenient to construct commit messages for use
> with 'rebase --autosquash'.  The resulting commit message will be
> "fixup! ..." where "..." is the subject line of the specified commit
> message.
>
> Example usage:
>  $ git commit --fixup HEAD~2

This is brilliant, I love it. I don't use 'autosquash' much atm
because I don't like messing with commit messages. This would make
using it much more convenient, thanks!

-- 
Cheers,

Sverre Rabbelier

Re: [PATCHv2 1/4] commit: --fixup option for use with rebase --autosquash

[Pat Notz]

On Tue, Sep 21, 2010 at 2:25 PM, Pat Notz <patnotz@gmail.com> wrote:
> This option makes it convenient to construct commit messages for use
> with 'rebase --autosquash'.  The resulting commit message will be
> "fixup! ..." where "..." is the subject line of the specified commit
> message.
>
> Example usage:
>  $ git commit --fixup HEAD~2
>
> Signed-off-by: Pat Notz <patnotz@gmail.com>
> ---
>  Documentation/git-commit.txt |   13 +++++++++----
>  builtin/commit.c             |   23 +++++++++++++++++++----
>  2 files changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 42fb1f5..3367f8f 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -9,10 +9,10 @@ SYNOPSIS
>  --------
>  [verse]
>  'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
> -          [(-c | -C) <commit>] [-F <file> | -m <msg>] [--reset-author]
> -          [--allow-empty] [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
> -          [--date=<date>] [--cleanup=<mode>] [--status | --no-status] [--]
> -          [[-i | -o ]<file>...]
> +          [(-c | -C | --fixup) <commit>] [-F <file> | -m <msg>]
> +          [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
> +          [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
> +          [--status | --no-status] [--] [[-i | -o ]<file>...]
>
>  DESCRIPTION
>  -----------
> @@ -70,6 +70,11 @@ OPTIONS
>        Like '-C', but with '-c' the editor is invoked, so that
>        the user can further edit the commit message.
>
> +--fixup=<commit>::
> +       Construct a commit message for use with `rebase --autosquash`.
> +       The commit message will be the subject line from the specified
> +       commit with a prefix of "fixup! ".
> +

I should add links to the git-rebase man page here.

[PATCHv2 2/4] t7500: add tests of commit --fixup

[Pat Notz]

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 t/t7500-commit.sh |   33 +++++++++++++++++++++++++++++++++
 1 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index aa9c577..7656ed4 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -215,4 +215,37 @@ test_expect_success 'Commit a message with --allow-empty-message' '
 	commit_msg_is "hello there"
 '
 
+commit_for_rebase_autosquash_setup() {
+	echo "first content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo &&
+	cat >log <<EOF
+target message subject line
+
+target message body line 1
+target message body line 2
+EOF
+	git commit -F log &&
+	echo "second content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo &&
+	git commit -m "intermediate commit" &&
+	echo "third content for testing commit messages for rebase --autosquash" >>foo &&
+	git add foo
+}
+
+test_expect_success 'commit --fixup provides correct one-line commit message' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --fixup HEAD~1 &&
+	commit_msg_is "fixup! target message subject line"
+'
+
+test_expect_success 'invalid message options when using --fixup' '
+	echo changes >>foo &&
+	echo "message" >log &&
+	git add foo &&
+	test_must_fail git commit --fixup HEAD~1 --C HEAD~2 &&
+	test_must_fail git commit --fixup HEAD~1 --c HEAD~2 &&
+	test_must_fail git commit --fixup HEAD~1 -m "cmdline message" &&
+	test_must_fail git commit --fixup HEAD~1 -F log
+'
+
 test_done
-- 
1.7.3

[PATCHv2 3/4] commit: --squash option for use with rebase --autosquash

[Pat Notz]

This option makes it convenient to construct commit messages for use
with 'rebase --autosquash'.  The resulting commit message will be
"squash! ..." where "..." is the subject line of the specified commit
message.  This option can be used with other commit message options
such as -m, -c, -C and -F.

If an editor is invoked (as with -c or -eF or no message options) the
commit message is seeded with the correctly formatted subject line.

Example usage:
  $ git commit --squash HEAD~2
  $ git commit --squash HEAD~2 -m "clever comment"
  $ git commit --squash HEAD~2 -F msgfile
  $ git commit --squash HEAD~2 -C deadbeef

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 Documentation/git-commit.txt |    8 +++++++-
 builtin/commit.c             |   37 +++++++++++++++++++++++++++++++++++--
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 3367f8f..b621dc4 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
-	   [(-c | -C | --fixup) <commit>] [-F <file> | -m <msg>]
+	   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
 	   [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
 	   [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
 	   [--status | --no-status] [--] [[-i | -o ]<file>...]
@@ -75,6 +75,12 @@ OPTIONS
 	The commit message will be the subject line from the specified
 	commit with a prefix of "fixup! ".
 
+--squash=<commit>::
+	Construct a commit message for use with `rebase --autosquash`.
+	The commit message subject line is taken from the specified
+	commit with a prefix of "squash! ".  Can be used with additional
+	commit message options (`-m`/`-c`/`-C`/`-F`).
+
 --reset-author::
 	When used with -C/-c/--amend options, declare that the
 	authorship of the resulting commit now belongs of the committer.
diff --git a/builtin/commit.c b/builtin/commit.c
index 0901616..d28b2ff 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -69,7 +69,7 @@ static enum {
 static const char *logfile, *force_author;
 static const char *template_file;
 static char *edit_message, *use_message;
-static char *fixup_message;
+static char *fixup_message, *squash_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
@@ -126,6 +126,7 @@ static struct option builtin_commit_options[] = {
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
 	OPT_STRING(0, "fixup", &fixup_message, "COMMIT", "use autosquash formatted message to fixup specified commit"),
+	OPT_STRING(0, "squash", &squash_message, "COMMIT", "use autosquash formatted message to squash specified commit"),
 	OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
 	OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
 	OPT_FILENAME('t', "template", &template_file, "use specified template file"),
@@ -567,6 +568,27 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (!no_verify && run_hook(index_file, "pre-commit", NULL))
 		return 0;
 
+	if (squash_message) {
+		/*
+		 * Insert the proper subject line before other commit
+		 * message options add their content.
+		 */
+		unsigned char sha1[20];
+		struct commit *commit;
+		struct pretty_print_context ctx = {0};
+
+		if (get_sha1(squash_message, sha1))
+			die("could not lookup commit %s", squash_message);
+		commit = lookup_commit_reference(sha1);
+		if (!commit || parse_commit(commit))
+			die("could not parse commit %s", squash_message);
+
+		if(use_message && strcmp(use_message, squash_message) == 0)
+			strbuf_addstr(&sb,"squash! ");
+		else
+			format_commit_message(commit, "squash! %s\n\n", &sb, &ctx);
+	}
+
 	if (message.len) {
 		strbuf_addbuf(&sb, &message);
 		hook_arg1 = "message";
@@ -620,6 +642,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	else if (in_merge)
 		hook_arg1 = "merge";
 
+	if (squash_message) {
+		/*
+		 * If squash_commit was used for the commit subject,
+		 * then we're possibly hijacking other commit log options.
+		 * Reset the hook args to tell the real story.
+		 */
+		hook_arg1 = "message";
+		hook_arg2 = "";
+	}
+
 	fp = fopen(git_path(commit_editmsg), "w");
 	if (fp == NULL)
 		die_errno("could not open '%s'", git_path(commit_editmsg));
@@ -891,7 +923,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
 		die("You have nothing to amend.");
 	if (amend && in_merge)
 		die("You are in the middle of a merge -- cannot amend.");
-
+	if (fixup_message && squash_message)
+		die("Options --squash and --fixup cannot be used together");
 	if (use_message)
 		f++;
 	if (edit_message)
-- 
1.7.3

Re: [PATCHv2 3/4] commit: --squash option for use with rebase --autosquash

[Pat Notz]

On Tue, Sep 21, 2010 at 2:25 PM, Pat Notz <patnotz@gmail.com> wrote:
> This option makes it convenient to construct commit messages for use
> with 'rebase --autosquash'.  The resulting commit message will be
> "squash! ..." where "..." is the subject line of the specified commit
> message.  This option can be used with other commit message options
> such as -m, -c, -C and -F.
>
> If an editor is invoked (as with -c or -eF or no message options) the
> commit message is seeded with the correctly formatted subject line.
>
> Example usage:
>  $ git commit --squash HEAD~2
>  $ git commit --squash HEAD~2 -m "clever comment"
>  $ git commit --squash HEAD~2 -F msgfile
>  $ git commit --squash HEAD~2 -C deadbeef
>
> Signed-off-by: Pat Notz <patnotz@gmail.com>
> ---
>  Documentation/git-commit.txt |    8 +++++++-
>  builtin/commit.c             |   37 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 42 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
> index 3367f8f..b621dc4 100644
> --- a/Documentation/git-commit.txt
> +++ b/Documentation/git-commit.txt
> @@ -9,7 +9,7 @@ SYNOPSIS
>  --------
>  [verse]
>  'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
> -          [(-c | -C | --fixup) <commit>] [-F <file> | -m <msg>]
> +          [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]
>           [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]
>           [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]
>           [--status | --no-status] [--] [[-i | -o ]<file>...]
> @@ -75,6 +75,12 @@ OPTIONS
>        The commit message will be the subject line from the specified
>        commit with a prefix of "fixup! ".
>
> +--squash=<commit>::
> +       Construct a commit message for use with `rebase --autosquash`.
> +       The commit message subject line is taken from the specified
> +       commit with a prefix of "squash! ".  Can be used with additional
> +       commit message options (`-m`/`-c`/`-C`/`-F`).
> +

Ditto - I should add links to the git-rebase man page here.

>  --reset-author::
>        When used with -C/-c/--amend options, declare that the
>        authorship of the resulting commit now belongs of the committer.
> diff --git a/builtin/commit.c b/builtin/commit.c
> index 0901616..d28b2ff 100644
> --- a/builtin/commit.c
> +++ b/builtin/commit.c
> @@ -69,7 +69,7 @@ static enum {
>  static const char *logfile, *force_author;
>  static const char *template_file;
>  static char *edit_message, *use_message;
> -static char *fixup_message;
> +static char *fixup_message, *squash_message;
>  static char *author_name, *author_email, *author_date;
>  static int all, edit_flag, also, interactive, only, amend, signoff;
>  static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;
> @@ -126,6 +126,7 @@ static struct option builtin_commit_options[] = {
>        OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit"),
>        OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
>        OPT_STRING(0, "fixup", &fixup_message, "COMMIT", "use autosquash formatted message to fixup specified commit"),
> +       OPT_STRING(0, "squash", &squash_message, "COMMIT", "use autosquash formatted message to squash specified commit"),
>        OPT_BOOLEAN(0, "reset-author", &renew_authorship, "the commit is authored by me now (used with -C-c/--amend)"),
>        OPT_BOOLEAN('s', "signoff", &signoff, "add Signed-off-by:"),
>        OPT_FILENAME('t', "template", &template_file, "use specified template file"),
> @@ -567,6 +568,27 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>        if (!no_verify && run_hook(index_file, "pre-commit", NULL))
>                return 0;
>
> +       if (squash_message) {
> +               /*
> +                * Insert the proper subject line before other commit
> +                * message options add their content.
> +                */
> +               unsigned char sha1[20];
> +               struct commit *commit;
> +               struct pretty_print_context ctx = {0};
> +
> +               if (get_sha1(squash_message, sha1))
> +                       die("could not lookup commit %s", squash_message);
> +               commit = lookup_commit_reference(sha1);
> +               if (!commit || parse_commit(commit))
> +                       die("could not parse commit %s", squash_message);
> +
> +               if(use_message && strcmp(use_message, squash_message) == 0)
> +                       strbuf_addstr(&sb,"squash! ");
> +               else
> +                       format_commit_message(commit, "squash! %s\n\n", &sb, &ctx);
> +       }
> +
>        if (message.len) {
>                strbuf_addbuf(&sb, &message);
>                hook_arg1 = "message";
> @@ -620,6 +642,16 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
>        else if (in_merge)
>                hook_arg1 = "merge";
>
> +       if (squash_message) {
> +               /*
> +                * If squash_commit was used for the commit subject,
> +                * then we're possibly hijacking other commit log options.
> +                * Reset the hook args to tell the real story.
> +                */
> +               hook_arg1 = "message";
> +               hook_arg2 = "";
> +       }
> +
>        fp = fopen(git_path(commit_editmsg), "w");
>        if (fp == NULL)
>                die_errno("could not open '%s'", git_path(commit_editmsg));
> @@ -891,7 +923,8 @@ static int parse_and_validate_options(int argc, const char *argv[],
>                die("You have nothing to amend.");
>        if (amend && in_merge)
>                die("You are in the middle of a merge -- cannot amend.");
> -
> +       if (fixup_message && squash_message)
> +               die("Options --squash and --fixup cannot be used together");
>        if (use_message)
>                f++;
>        if (edit_message)
> --
> 1.7.3
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCHv2 4/4] t7500: add tests of commit --squash

[Pat Notz]

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 t/t7500-commit.sh |   57 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 57 insertions(+), 0 deletions(-)

diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index 7656ed4..a2406f4 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -238,10 +238,67 @@ test_expect_success 'commit --fixup provides correct one-line commit message' '
 	commit_msg_is "fixup! target message subject line"
 '
 
+test_expect_success 'commit --squash works with -F' '
+	commit_for_rebase_autosquash_setup &&
+	echo "log message from file" >msgfile
+	git commit --squash HEAD~1 -F msgfile  &&
+	commit_msg_is "squash! target message subject linelog message from file"
+'
+
+test_expect_success 'commit --squash works with -m' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --squash HEAD~1 -m "foo bar\nbaz" &&
+	commit_msg_is "squash! target message subject linefoo bar\nbaz"
+'
+
+test_expect_success 'commit --squash works with -C' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --squash HEAD~1 -C HEAD &&
+	commit_msg_is "squash! target message subject lineintermediate commit"
+'
+
+cat >editor <<\EOF
+#!/bin/sh
+sed -e "s/intermediate/edited/g" <"$1" >"$1-"
+mv "$1-" "$1"
+EOF
+chmod 755 editor
+
+test_expect_success 'commit --squash works with -c' '
+	commit_for_rebase_autosquash_setup &&
+	EDITOR=./editor git commit --squash HEAD~1 -c HEAD &&
+	commit_msg_is "squash! target message subject lineedited commit"
+'
+
+test_expect_success 'commit --squash works with -C for same commit' '
+	commit_for_rebase_autosquash_setup &&
+	git commit --squash HEAD -C HEAD &&
+	commit_msg_is "squash! intermediate commit"
+'
+
+test_expect_success 'commit --squash works with -c for same commit' '
+	commit_for_rebase_autosquash_setup &&
+	EDITOR=./editor git commit --squash HEAD -c HEAD &&
+	commit_msg_is "squash! edited commit"
+'
+
+cat >editor <<\EOF
+#!/bin/sh
+echo "additional body content" >>"$1"
+EOF
+chmod 755 editor
+
+test_expect_success 'commit --squash works with editor' '
+	commit_for_rebase_autosquash_setup &&
+	EDITOR=./editor git commit --squash HEAD~1 &&
+	commit_msg_is "squash! target message subject lineadditional body content"
+'
+
 test_expect_success 'invalid message options when using --fixup' '
 	echo changes >>foo &&
 	echo "message" >log &&
 	git add foo &&
+	test_must_fail git commit --fixup HEAD~1 --squash HEAD~2 &&
 	test_must_fail git commit --fixup HEAD~1 --C HEAD~2 &&
 	test_must_fail git commit --fixup HEAD~1 --c HEAD~2 &&
 	test_must_fail git commit --fixup HEAD~1 -m "cmdline message" &&
-- 
1.7.3

Re: [PATCHv2 4/4] t7500: add tests of commit --squash

[Ævar Arnfjörð Bjarmason]

On Tue, Sep 21, 2010 at 20:25, Pat Notz <patnotz@gmail.com> wrote:

> +cat >editor <<\EOF
> +#!/bin/sh
> +sed -e "s/intermediate/edited/g" <"$1" >"$1-"
> +mv "$1-" "$1"
> +EOF
> +chmod 755 editor
> +
> +test_expect_success 'commit --squash works with -c' '
> +       commit_for_rebase_autosquash_setup &&
> +       EDITOR=./editor git commit --squash HEAD~1 -c HEAD &&
> +       commit_msg_is "squash! target message subject lineedited commit"
> +'

Why not put the editor in t/t7500/ and use test_set_editor() like the
other tests?

Re: [PATCHv2 4/4] t7500: add tests of commit --squash

[Pat Notz]

On Tue, Sep 21, 2010 at 2:36 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
>
> On Tue, Sep 21, 2010 at 20:25, Pat Notz <patnotz@gmail.com> wrote:
>
> > +cat >editor <<\EOF
> > +#!/bin/sh
> > +sed -e "s/intermediate/edited/g" <"$1" >"$1-"
> > +mv "$1-" "$1"
> > +EOF
> > +chmod 755 editor
> > +
> > +test_expect_success 'commit --squash works with -c' '
> > +       commit_for_rebase_autosquash_setup &&
> > +       EDITOR=./editor git commit --squash HEAD~1 -c HEAD &&
> > +       commit_msg_is "squash! target message subject lineedited commit"
> > +'
>
> Why not put the editor in t/t7500/ and use test_set_editor() like the
> other tests?

The real reason is that I'm new enough that I wasn't aware of this
pattern.  I saw what was done in t7501-commit.sh and followed along.
I missed the use of test_set_editor() right there in t7500-commit.sh.
Doh!

I can certainly do that if it's preferred.  I must say, though, that I
find it odd to put test inputs in a separate file in a separate
directory from where the test transforms those into expected outputs.
To see what the test is doing you have to load both files and trace
through it.

Still, I'd be happy to change do this if that's the preferred way.

Re: [PATCHv2 4/4] t7500: add tests of commit --squash

[Ævar Arnfjörð Bjarmason]

On Wed, Sep 22, 2010 at 17:59, Pat Notz <patnotz@gmail.com> wrote:
> On Tue, Sep 21, 2010 at 2:36 PM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>>
>> On Tue, Sep 21, 2010 at 20:25, Pat Notz <patnotz@gmail.com> wrote:
>>
>> > +cat >editor <<\EOF
>> > +#!/bin/sh
>> > +sed -e "s/intermediate/edited/g" <"$1" >"$1-"
>> > +mv "$1-" "$1"
>> > +EOF
>> > +chmod 755 editor
>> > +
>> > +test_expect_success 'commit --squash works with -c' '
>> > +       commit_for_rebase_autosquash_setup &&
>> > +       EDITOR=./editor git commit --squash HEAD~1 -c HEAD &&
>> > +       commit_msg_is "squash! target message subject lineedited commit"
>> > +'
>>
>> Why not put the editor in t/t7500/ and use test_set_editor() like the
>> other tests?
>
> The real reason is that I'm new enough that I wasn't aware of this
> pattern.  I saw what was done in t7501-commit.sh and followed along.
> I missed the use of test_set_editor() right there in t7500-commit.sh.
> Doh!
>
> I can certainly do that if it's preferred.  I must say, though, that I
> find it odd to put test inputs in a separate file in a separate
> directory from where the test transforms those into expected outputs.
> To see what the test is doing you have to load both files and trace
> through it.
>
> Still, I'd be happy to change do this if that's the preferred way.

It's a bit odd, but it's best to following existing style within a
test. Then maybe submit fixup patches to fix the whole thing later.

Re: [PATCHv2 4/4] t7500: add tests of commit --squash

[Pat Notz]

On Wed, Sep 22, 2010 at 12:12 PM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> On Wed, Sep 22, 2010 at 17:59, Pat Notz <patnotz@gmail.com> wrote:
>> On Tue, Sep 21, 2010 at 2:36 PM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>>
>>> On Tue, Sep 21, 2010 at 20:25, Pat Notz <patnotz@gmail.com> wrote:
>>>
>>> > +cat >editor <<\EOF
>>> > +#!/bin/sh
>>> > +sed -e "s/intermediate/edited/g" <"$1" >"$1-"
>>> > +mv "$1-" "$1"
>>> > +EOF
>>> > +chmod 755 editor
>>> > +
>>> > +test_expect_success 'commit --squash works with -c' '
>>> > +       commit_for_rebase_autosquash_setup &&
>>> > +       EDITOR=./editor git commit --squash HEAD~1 -c HEAD &&
>>> > +       commit_msg_is "squash! target message subject lineedited commit"
>>> > +'
>>>
>>> Why not put the editor in t/t7500/ and use test_set_editor() like the
>>> other tests?
>>
>> The real reason is that I'm new enough that I wasn't aware of this
>> pattern.  I saw what was done in t7501-commit.sh and followed along.
>> I missed the use of test_set_editor() right there in t7500-commit.sh.
>> Doh!
>>
>> I can certainly do that if it's preferred.  I must say, though, that I
>> find it odd to put test inputs in a separate file in a separate
>> directory from where the test transforms those into expected outputs.
>> To see what the test is doing you have to load both files and trace
>> through it.
>>
>> Still, I'd be happy to change do this if that's the preferred way.
>
> It's a bit odd, but it's best to following existing style within a
> test. Then maybe submit fixup patches to fix the whole thing later.
>

Yeah, there's certainly value in doing that.  I'll follow-up with a v3
and include the documentation changes I noticed in the other patches.