* [PATCHv4 0/4] Add commit message options for rebase --autosquash
@ 2010-09-23 17:14 Pat Notz
2010-09-23 17:14 ` [PATCHv4 1/4] commit: --fixup option for use with " Pat Notz
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Pat Notz @ 2010-09-23 17:14 UTC (permalink / raw)
To: git
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'.
The 2nd iteration addressed 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.
The 3rd iteration added documentation links from --squash and --fixup
to git-rebase(1) and updated the tests to conform to established style
(editor script moved into t7500/ directory).
This 4th iteration adds missing '&&' to a couple of lines in the tests.
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 | 21 +++++++++--
builtin/commit.c | 58 +++++++++++++++++++++++++++---
t/t7500-commit.sh | 80 ++++++++++++++++++++++++++++++++++++++++++
t/t7500/edit-content | 4 ++
4 files changed, 154 insertions(+), 9 deletions(-)
create mode 100755 t/t7500/edit-content
--
1.7.3
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCHv4 1/4] commit: --fixup option for use with rebase --autosquash
2010-09-23 17:14 [PATCHv4 0/4] Add commit message options for rebase --autosquash Pat Notz
@ 2010-09-23 17:14 ` Pat Notz
2010-09-23 17:56 ` Junio C Hamano
2010-09-23 17:14 ` [PATCHv4 2/4] t7500: add tests of commit --fixup Pat Notz
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Pat Notz @ 2010-09-23 17:14 UTC (permalink / raw)
To: git
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 | 14 ++++++++++----
builtin/commit.c | 23 +++++++++++++++++++----
2 files changed, 29 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 42fb1f5..f4a2b8c 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,12 @@ 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! ". See linkgit:git-rebase[1]
+ for details.
+
--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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCHv4 2/4] t7500: add tests of commit --fixup
2010-09-23 17:14 [PATCHv4 0/4] Add commit message options for rebase --autosquash Pat Notz
2010-09-23 17:14 ` [PATCHv4 1/4] commit: --fixup option for use with " Pat Notz
@ 2010-09-23 17:14 ` Pat Notz
2010-09-23 17:42 ` Junio C Hamano
2010-09-23 17:14 ` [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash Pat Notz
2010-09-23 17:14 ` [PATCHv4 4/4] t7500: add tests of commit --squash Pat Notz
3 siblings, 1 reply; 9+ messages in thread
From: Pat Notz @ 2010-09-23 17:14 UTC (permalink / raw)
To: git
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..db82264 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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash
2010-09-23 17:14 [PATCHv4 0/4] Add commit message options for rebase --autosquash Pat Notz
2010-09-23 17:14 ` [PATCHv4 1/4] commit: --fixup option for use with " Pat Notz
2010-09-23 17:14 ` [PATCHv4 2/4] t7500: add tests of commit --fixup Pat Notz
@ 2010-09-23 17:14 ` Pat Notz
2010-09-23 20:39 ` Junio C Hamano
2010-09-23 17:14 ` [PATCHv4 4/4] t7500: add tests of commit --squash Pat Notz
3 siblings, 1 reply; 9+ messages in thread
From: Pat Notz @ 2010-09-23 17:14 UTC (permalink / raw)
To: git
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 | 9 ++++++++-
builtin/commit.c | 37 +++++++++++++++++++++++++++++++++++--
2 files changed, 43 insertions(+), 3 deletions(-)
diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index f4a2b8c..6e4c220 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>...]
@@ -76,6 +76,13 @@ OPTIONS
commit with a prefix of "fixup! ". See linkgit:git-rebase[1]
for details.
+--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`). See
+ linkgit:git-rebase[1] for details.
+
--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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCHv4 4/4] t7500: add tests of commit --squash
2010-09-23 17:14 [PATCHv4 0/4] Add commit message options for rebase --autosquash Pat Notz
` (2 preceding siblings ...)
2010-09-23 17:14 ` [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash Pat Notz
@ 2010-09-23 17:14 ` Pat Notz
3 siblings, 0 replies; 9+ messages in thread
From: Pat Notz @ 2010-09-23 17:14 UTC (permalink / raw)
To: git
Signed-off-by: Pat Notz <patnotz@gmail.com>
---
t/t7500-commit.sh | 47 +++++++++++++++++++++++++++++++++++++++++++++++
t/t7500/edit-content | 4 ++++
2 files changed, 51 insertions(+), 0 deletions(-)
create mode 100755 t/t7500/edit-content
diff --git a/t/t7500-commit.sh b/t/t7500-commit.sh
index db82264..2fd72be 100755
--- a/t/t7500-commit.sh
+++ b/t/t7500-commit.sh
@@ -238,10 +238,57 @@ 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"
+'
+
+test_expect_success 'commit --squash works with -c' '
+ commit_for_rebase_autosquash_setup &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/edit-content &&
+ 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 &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/edit-content &&
+ git commit --squash HEAD -c HEAD &&
+ commit_msg_is "squash! edited commit"
+'
+
+test_expect_success 'commit --squash works with editor' '
+ commit_for_rebase_autosquash_setup &&
+ test_set_editor "$TEST_DIRECTORY"/t7500/add-content &&
+ git commit --squash HEAD~1 &&
+ commit_msg_is "squash! target message subject linecommit message"
+'
+
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" &&
diff --git a/t/t7500/edit-content b/t/t7500/edit-content
new file mode 100755
index 0000000..08db9fd
--- /dev/null
+++ b/t/t7500/edit-content
@@ -0,0 +1,4 @@
+#!/bin/sh
+sed -e "s/intermediate/edited/g" <"$1" >"$1-"
+mv "$1-" "$1"
+exit 0
--
1.7.3
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCHv4 2/4] t7500: add tests of commit --fixup
2010-09-23 17:14 ` [PATCHv4 2/4] t7500: add tests of commit --fixup Pat Notz
@ 2010-09-23 17:42 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2010-09-23 17:42 UTC (permalink / raw)
To: Pat Notz; +Cc: git
"Pat Notz" <patnotz@gmail.com> writes:
> 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..db82264 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() {
A SP after "_setup", i.e. "..._setup () {".
> + echo "first content for testing commit messages for rebase --autosquash" >>foo &&
Did you really need this long line here?
> + 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"
> +'
What should be the right output when "target message subject line" has
some metacharacters, i.e. "." (regexp) or "?" (glob)?
Don't we also want to make sure that "rebase --autosquash" correctly groks
the history you prepared in this test?
> +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 &&
Double dashes before "C" and "c" look fishy. Don't.
> + 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv4 1/4] commit: --fixup option for use with rebase --autosquash
2010-09-23 17:14 ` [PATCHv4 1/4] commit: --fixup option for use with " Pat Notz
@ 2010-09-23 17:56 ` Junio C Hamano
2010-09-23 18:27 ` Pat Notz
0 siblings, 1 reply; 9+ messages in thread
From: Junio C Hamano @ 2010-09-23 17:56 UTC (permalink / raw)
To: Pat Notz; +Cc: git
"Pat Notz" <patnotz@gmail.com> writes:
> + } 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";
I notice that the above is a half-copy-and-paste from "if (use_message)"
codepath that handles -c/-C. A few issues to think about (i.e. not
complaints; I haven't thought about them myself):
(1) Is it worth refactoring the original instead of copying;
(2) What happens/should happen when the original commit is encoded
differently from the current commit encoding? -c/-C codepath takes
pains to re-encode. Should we do so somewhere in this codepath, too?
(3) If the answer to (2) is "Yes", notice that format_commit_message()
does not re-encode the commit log message ("log" output codepath uses
pretty.c::pretty_print_commit(), which reencodes for log output
encoding). Maybe we need an option to tell format_commit_message()
to do so?
The last is not exactly an issue this patch alone should address, but I
thought I'd better mention it anyway.
My knee-jerk answers to the above are:
(1) The first handful of lines in this new "if (fixup_message)" codeblock
up to "die" might want to use a helper function shared with the
existing "if (use_message)" codepath;
(2) We probably want to re-encode to the log output encoding the string
we receive from format_commit_message() in this codepath.
(3) No need yet.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv4 1/4] commit: --fixup option for use with rebase --autosquash
2010-09-23 17:56 ` Junio C Hamano
@ 2010-09-23 18:27 ` Pat Notz
0 siblings, 0 replies; 9+ messages in thread
From: Pat Notz @ 2010-09-23 18:27 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
On Thu, Sep 23, 2010 at 11:56 AM, Junio C Hamano <gitster@pobox.com> wrote:
> "Pat Notz" <patnotz@gmail.com> writes:
>
>> + } 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";
>
>
> I notice that the above is a half-copy-and-paste from "if (use_message)"
> codepath that handles -c/-C. A few issues to think about (i.e. not
> complaints; I haven't thought about them myself):
>
> (1) Is it worth refactoring the original instead of copying;
>
> (2) What happens/should happen when the original commit is encoded
> differently from the current commit encoding? -c/-C codepath takes
> pains to re-encode. Should we do so somewhere in this codepath, too?
>
Yes, this was the concern I expressed in the v1 series patch. I'm
getting more comfortable with the code so I'll look into re-encoding
appropriately.
> (3) If the answer to (2) is "Yes", notice that format_commit_message()
> does not re-encode the commit log message ("log" output codepath uses
> pretty.c::pretty_print_commit(), which reencodes for log output
> encoding). Maybe we need an option to tell format_commit_message()
> to do so?
>
> The last is not exactly an issue this patch alone should address, but I
> thought I'd better mention it anyway.
>
> My knee-jerk answers to the above are:
>
> (1) The first handful of lines in this new "if (fixup_message)" codeblock
> up to "die" might want to use a helper function shared with the
> existing "if (use_message)" codepath;
Agreed. I'll factor out the duplication.
>
> (2) We probably want to re-encode to the log output encoding the string
> we receive from format_commit_message() in this codepath.
Will do.
>
> (3) No need yet.
>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash
2010-09-23 17:14 ` [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash Pat Notz
@ 2010-09-23 20:39 ` Junio C Hamano
0 siblings, 0 replies; 9+ messages in thread
From: Junio C Hamano @ 2010-09-23 20:39 UTC (permalink / raw)
To: Pat Notz; +Cc: git
"Pat Notz" <patnotz@gmail.com> writes:
> + if(use_message && strcmp(use_message, squash_message) == 0)
"if (use_message && !strcmp(..., ...))"
Otherwise this patch looks sane to me.
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-09-23 20:39 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-09-23 17:14 [PATCHv4 0/4] Add commit message options for rebase --autosquash Pat Notz
2010-09-23 17:14 ` [PATCHv4 1/4] commit: --fixup option for use with " Pat Notz
2010-09-23 17:56 ` Junio C Hamano
2010-09-23 18:27 ` Pat Notz
2010-09-23 17:14 ` [PATCHv4 2/4] t7500: add tests of commit --fixup Pat Notz
2010-09-23 17:42 ` Junio C Hamano
2010-09-23 17:14 ` [PATCHv4 3/4] commit: --squash option for use with rebase --autosquash Pat Notz
2010-09-23 20:39 ` Junio C Hamano
2010-09-23 17:14 ` [PATCHv4 4/4] t7500: add tests of commit --squash Pat Notz
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).