{"thread":{"id":"27289","subject":"[PATCH v3 0/3] Git commit --patch (again)","startedAt":"2011-05-07T05:59:58Z","lastAt":"2011-05-10T21:01:48Z","messageCount":21,"participants":["conrad.irwin@gmail.com","Sverre Rabbelier","Valentin Haenel","Conrad Irwin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":3,"patchTotal":3},"messages":[{"id":"167307","messageId":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":null,"subject":"[PATCH v3 0/3] Git commit --patch (again)","fromName":"","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T05:59:58Z","receivedAt":"2011-05-07T05:59:58Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"From: Conrad Irwin <conrad.irwin@gmail.com>\n\nHi all,\n\nI've rebased my support for git commit -p onto the current master\nbranch. I've posted it to the list twice before [1][2].\n\nThe purpose of the branch is to add the -p/--patch flag, used by other\ncommands such as git add and git checkout, to git commit. This makes it\nmuch easier, and more convenient, to construct well-formed patches after\na session of writing code.\n\nThis branch also includes a fix for git-commit --interactive, so that if\nthe committer aborts after selecting hunks, the index is unchanged (just\nas specifying filenames in the arguments doesn't permanently add them to\nthe index).\n\nFeedback (of any variety) would be much appreciated,\n\nConrad\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/164193\n[2] http://thread.gmane.org/gmane.comp.version-control.git/165951\n\nConrad Irwin (3):\n  Use a temporary index for git commit --interactive\n  Allow git commit --interactive with paths\n  Add support for -p/--patch to git-commit\n\n Documentation/git-commit.txt |   24 ++++++++++++++-------\n builtin/add.c                |    6 ++--\n builtin/commit.c             |   46 +++++++++++++++++++++++++++++++-----------\n commit.h                     |    2 +-\n 4 files changed, 54 insertions(+), 24 deletions(-)\n\n-- \n1.7.5.188.g4817\n"},{"id":"167309","messageId":"1304748001-17982-2-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","subject":"[PATCH v3 1/3] Use a temporary index for git commit --interactive","fromName":"","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T05:59:59Z","receivedAt":"2011-05-07T05:59:59Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"From: Conrad Irwin <conrad.irwin@gmail.com>\n\nChange the behaviour of git commit --interactive so that when you abort\nthe commit (by leaving the commit message empty) the index remains\nunchanged.\n\nHitherto an aborted commit --interactive has added the selected hunks to\nthe index regardless of whether the commit succeeded or not.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-commit.txt |    3 ++-\n builtin/commit.c             |   36 ++++++++++++++++++++++++++++--------\n 2 files changed, 30 insertions(+), 9 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex d0534b8..ed50271 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -41,7 +41,8 @@ The content to be added can be specified in several ways:\n \n 5. by using the --interactive switch with the 'commit' command to decide one\n    by one which files should be part of the commit, before finalizing the\n-   operation.  Currently, this is done by invoking 'git add --interactive'.\n+   operation.  Currently, this is done by invoking 'git add --interactive'\n+   on a temporary index.\n \n The `--dry-run` option can be used to obtain a\n summary of what is included by any of the above for the next\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 67757e9..636aea6 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -336,18 +336,11 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tint fd;\n \tstruct string_list partial;\n \tconst char **pathspec = NULL;\n+\tchar *old_index_env = NULL;\n \tint refresh_flags = REFRESH_QUIET;\n \n \tif (is_status)\n \t\trefresh_flags |= REFRESH_UNMERGED;\n-\tif (interactive) {\n-\t\tif (interactive_add(argc, argv, prefix) != 0)\n-\t\t\tdie(_(\"interactive add failed\"));\n-\t\tif (read_cache_preload(NULL) < 0)\n-\t\t\tdie(_(\"index file corrupt\"));\n-\t\tcommit_style = COMMIT_AS_IS;\n-\t\treturn get_index_file();\n-\t}\n \n \tif (*argv)\n \t\tpathspec = get_pathspec(prefix, argv);\n@@ -355,6 +348,33 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \tif (read_cache_preload(pathspec) < 0)\n \t\tdie(_(\"index file corrupt\"));\n \n+\tif (interactive) {\n+\t\tfd = hold_locked_index(&index_lock, 1);\n+\n+\t\trefresh_cache_or_die(refresh_flags);\n+\n+\t\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t    close_lock_file(&index_lock))\n+\t\t\tdie(_(\"unable to create temporary index\"));\n+\n+\t\told_index_env = getenv(INDEX_ENVIRONMENT);\n+\t\tsetenv(INDEX_ENVIRONMENT, index_lock.filename, 1);\n+\n+\t\tif (interactive_add(argc, argv, prefix) != 0)\n+\t\t\tdie(_(\"interactive add failed\"));\n+\n+\t\tif (old_index_env && *old_index_env)\n+\t\t\tsetenv(INDEX_ENVIRONMENT, old_index_env, 1);\n+\t\telse\n+\t\t\tunsetenv(INDEX_ENVIRONMENT);\n+\n+\t\tdiscard_cache();\n+\t\tread_cache_from(index_lock.filename);\n+\n+\t\tcommit_style = COMMIT_NORMAL;\n+\t\treturn index_lock.filename;\n+\t}\n+\n \t/*\n \t * Non partial, non as-is commit.\n \t *\n-- \n1.7.5.188.g4817\n"},{"id":"167308","messageId":"1304748001-17982-3-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","subject":"[PATCH v3 2/3] Allow git commit --interactive with paths","fromName":"","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T06:00:00Z","receivedAt":"2011-05-07T06:00:00Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"From: Conrad Irwin <conrad.irwin@gmail.com>\n\nMake git commit --interactive feel more like git add --interactive by\nallowing the user to restrict the list of files they have to deal with.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n builtin/commit.c |    2 --\n 1 files changed, 0 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 636aea6..7707af8 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1084,8 +1084,6 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \n \tif (all && argc > 0)\n \t\tdie(_(\"Paths with -a does not make sense.\"));\n-\telse if (interactive && argc > 0)\n-\t\tdie(_(\"Paths with --interactive does not make sense.\"));\n \n \tif (null_termination && status_format == STATUS_FORMAT_LONG)\n \t\tstatus_format = STATUS_FORMAT_PORCELAIN;\n-- \n1.7.5.188.g4817\n"},{"id":"167310","messageId":"1304748001-17982-4-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","subject":"[PATCH v3 3/3] Add support for -p/--patch to git-commit","fromName":"","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T06:00:01Z","receivedAt":"2011-05-07T06:00:01Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"From: Conrad Irwin <conrad.irwin@gmail.com>\n\nThe --interactive flag is already shared by git add and git commit,\nshare the -p and --patch flags too.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-commit.txt |   25 ++++++++++++++++---------\n builtin/add.c                |    6 +++---\n builtin/commit.c             |   10 +++++++---\n commit.h                     |    2 +-\n 4 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex ed50271..66918c7 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -8,11 +8,12 @@ git-commit - Record changes to the repository\n SYNOPSIS\n --------\n [verse]\n-'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]\n-\t   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]\n-\t   [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]\n-\t   [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]\n-\t   [--status | --no-status] [-i | -o] [--] [<file>...]\n+'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n+\t   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]\n+\t   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\n+\t   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\n+\t   [--date=<date>] [--cleanup=<mode>] [--status | --no-status]\n+\t   [-i | -o] [--] [<file>...]\n \n DESCRIPTION\n -----------\n@@ -39,10 +40,10 @@ The content to be added can be specified in several ways:\n    that have been removed from the working tree, and then perform the\n    actual commit;\n \n-5. by using the --interactive switch with the 'commit' command to decide one\n-   by one which files should be part of the commit, before finalizing the\n-   operation.  Currently, this is done by invoking 'git add --interactive'\n-   on a temporary index.\n+5. by using the --interactive or --patch switches with the 'commit' command\n+   to decide one by one which files or hunks should be part of the commit,\n+   before finalizing the operation.  Currently, this is done by invoking\n+   'git add --interactive' or 'git add --patch' on a temporary index.\n \n The `--dry-run` option can be used to obtain a\n summary of what is included by any of the above for the next\n@@ -60,6 +61,12 @@ OPTIONS\n \tbeen modified and deleted, but new files you have not\n \ttold git about are not affected.\n \n+-p::\n+--patch::\n+\tUse the interactive patch selection interface to chose\n+\twhich changes to commit. See linkgit:git-add[1] for\n+\tdetails.\n+\n -C <commit>::\n --reuse-message=<commit>::\n \tTake an existing commit object, and reuse the log message\ndiff --git a/builtin/add.c b/builtin/add.c\nindex d39a6ab..f02524b 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -241,7 +241,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \treturn status;\n }\n \n-int interactive_add(int argc, const char **argv, const char *prefix)\n+int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n {\n \tconst char **pathspec = NULL;\n \n@@ -252,7 +252,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)\n \t}\n \n \treturn run_add_interactive(NULL,\n-\t\t\t\t   patch_interactive ? \"--patch\" : NULL,\n+\t\t\t\t   patch ? \"--patch\" : NULL,\n \t\t\t\t   pathspec);\n }\n \n@@ -377,7 +377,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive)\n-\t\texit(interactive_add(argc - 1, argv + 1, prefix));\n+\t\texit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));\n \n \tif (edit_interactive)\n \t\treturn(edit_patch(argc, argv, prefix));\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 7707af8..008c1ec 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -83,7 +83,7 @@ static const char *template_file;\n static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n static char *fixup_message, *squash_message;\n-static int all, edit_flag, also, interactive, only, amend, signoff;\n+static int all, edit_flag, also, interactive, patch_interactive, only, amend, signoff;\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg;\n@@ -152,6 +152,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN('a', \"all\", &all, \"commit all changed files\"),\n \tOPT_BOOLEAN('i', \"include\", &also, \"add specified files to index for commit\"),\n \tOPT_BOOLEAN(0, \"interactive\", &interactive, \"interactively add files\"),\n+\tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactively add changes\"),\n \tOPT_BOOLEAN('o', \"only\", &only, \"commit only specified files\"),\n \tOPT_BOOLEAN('n', \"no-verify\", &no_verify, \"bypass pre-commit hook\"),\n \tOPT_BOOLEAN(0, \"dry-run\", &dry_run, \"show what would be committed\"),\n@@ -360,7 +361,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t\told_index_env = getenv(INDEX_ENVIRONMENT);\n \t\tsetenv(INDEX_ENVIRONMENT, index_lock.filename, 1);\n \n-\t\tif (interactive_add(argc, argv, prefix) != 0)\n+\t\tif (interactive_add(argc, argv, prefix, patch_interactive) != 0)\n \t\t\tdie(_(\"interactive add failed\"));\n \n \t\tif (old_index_env && *old_index_env)\n@@ -1061,8 +1062,11 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tauthor_message_buffer = read_commit_message(author_message);\n \t}\n \n+\tif (patch_interactive)\n+\t\tinteractive = 1;\n+\n \tif (!!also + !!only + !!all + !!interactive > 1)\n-\t\tdie(_(\"Only one of --include/--only/--all/--interactive can be used.\"));\n+\t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tif (argc == 0 && only && amend)\ndiff --git a/commit.h b/commit.h\nindex b3c3bb7..43940e2 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -161,7 +161,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,\n int is_descendant_of(struct commit *, struct commit_list *);\n int in_merge_bases(struct commit *, struct commit **, int);\n \n-extern int interactive_add(int argc, const char **argv, const char *prefix);\n+extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);\n extern int run_add_interactive(const char *revision, const char *patch_mode,\n \t\t\t       const char **pathspec);\n \n-- \n1.7.5.188.g4817\n"},{"id":"167313","messageId":"BANLkTi=N2-uNb1mjOdCGbdn-DfF77D_rfw@mail.gmail.com","threadId":"27289","inReplyTo":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2011-05-07T09:49:10Z","receivedAt":"2011-05-07T09:49:10Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Sat, May 7, 2011 at 07:59,  <conrad.irwin@gmail.com> wrote:\n> I've rebased my support for git commit -p onto the current master\n> branch. I've posted it to the list twice before [1][2].\n\nI thought this got merged and have been disappointed a few times when\n'git commit -p' didn't work. So I'm in favor :)\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"167318","messageId":"20110507104613.GA20375@kudu.in-berlin.de","threadId":"27289","inReplyTo":"1304748001-17982-4-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v3 3/3] Add support for -p/--patch to git-commit","fromName":"Valentin Haenel","fromEmail":"valentin@fsfe.org","sentAt":"2011-05-07T10:46:13Z","receivedAt":"2011-05-07T10:46:13Z","isPatch":true,"sender":{"key":"valentin@fsfe.org","avatar":null},"body":"Being a big fan of the --patch options for various commands, I like this\none.\n\nSince it uses the add--interactive.perl stuff, it would also be affacted\nby 'interactive.singlekey'. So it might be worth mentioning that in\nconfig.txt for sake of completeness. I currently have some patches that\nmodify config.txt in this regard, cooking in next, see: 46b522c\n\nV-\n"},{"id":"167326","messageId":"BANLkTi=Y5o=KP1LnkKqGq31Sqfn-ZZCGNA@mail.gmail.com","threadId":"27289","inReplyTo":"20110507104613.GA20375@kudu.in-berlin.de","subject":"Re: [PATCH v3 3/3] Add support for -p/--patch to git-commit","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T17:55:05Z","receivedAt":"2011-05-07T17:55:05Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"On Sat, May 7, 2011 at 3:46 AM, Valentin Haenel <valentin@fsfe.org> wrote:\n> Since it uses the add--interactive.perl stuff, it would also be affacted\n> by 'interactive.singlekey'. So it might be worth mentioning that in\n> config.txt for sake of completeness. I currently have some patches that\n> modify config.txt in this regard, cooking in next, see: 46b522c\n>\n\nOk, I've re-rolled the 3rd patch to use documentation in a similar\nmanner to yours for consistency. (It actually links to the\ndocumentation for git-add as opposed to hinting that more information\nmay be found there).\n\nI've also created a fourth patch that applies to the merge of this\npatch set with vh/config-interactive-singlekey-doc to add git-commit\nto the list of commands affected by interactive.singlekey.\n\nThanks,\nConrad\n"},{"id":"167328","messageId":"1304791087-2965-1-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"BANLkTi=Y5o=KP1LnkKqGq31Sqfn-ZZCGNA@mail.gmail.com","subject":"[PATCH v4 3/3] Add support for -p/--patch to git-commit","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T17:58:07Z","receivedAt":"2011-05-07T17:58:07Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"The --interactive flag is already shared by git add and git commit,\nshare the -p and --patch flags too.\n\nSigned-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/git-commit.txt |   25 ++++++++++++++++---------\n builtin/add.c                |    6 +++---\n builtin/commit.c             |   10 +++++++---\n commit.h                     |    2 +-\n 4 files changed, 27 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex ed50271..7951cb7 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -8,11 +8,12 @@ git-commit - Record changes to the repository\n SYNOPSIS\n --------\n [verse]\n-'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]\n-\t   [(-c | -C | --fixup | --squash) <commit>] [-F <file> | -m <msg>]\n-\t   [--reset-author] [--allow-empty] [--allow-empty-message] [--no-verify]\n-\t   [-e] [--author=<author>] [--date=<date>] [--cleanup=<mode>]\n-\t   [--status | --no-status] [-i | -o] [--] [<file>...]\n+'git commit' [-a | --interactive | --patch] [-s] [-v] [-u<mode>] [--amend]\n+\t   [--dry-run] [(-c | -C | --fixup | --squash) <commit>]\n+\t   [-F <file> | -m <msg>] [--reset-author] [--allow-empty]\n+\t   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]\n+\t   [--date=<date>] [--cleanup=<mode>] [--status | --no-status]\n+\t   [-i | -o] [--] [<file>...]\n \n DESCRIPTION\n -----------\n@@ -39,10 +40,10 @@ The content to be added can be specified in several ways:\n    that have been removed from the working tree, and then perform the\n    actual commit;\n \n-5. by using the --interactive switch with the 'commit' command to decide one\n-   by one which files should be part of the commit, before finalizing the\n-   operation.  Currently, this is done by invoking 'git add --interactive'\n-   on a temporary index.\n+5. by using the --interactive or --patch switches with the 'commit' command\n+   to decide one by one which files or hunks should be part of the commit,\n+   before finalizing the operation. See the ``Interactive Mode`` section of\n+   linkgit:git-add[1] to learn how to operate these modes.\n \n The `--dry-run` option can be used to obtain a\n summary of what is included by any of the above for the next\n@@ -60,6 +61,12 @@ OPTIONS\n \tbeen modified and deleted, but new files you have not\n \ttold git about are not affected.\n \n+-p::\n+--patch::\n+\tUse the interactive patch selection interface to chose\n+\twhich changes to commit. See linkgit:git-add[1] for\n+\tdetails.\n+\n -C <commit>::\n --reuse-message=<commit>::\n \tTake an existing commit object, and reuse the log message\ndiff --git a/builtin/add.c b/builtin/add.c\nindex d39a6ab..f02524b 100644\n--- a/builtin/add.c\n+++ b/builtin/add.c\n@@ -241,7 +241,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n \treturn status;\n }\n \n-int interactive_add(int argc, const char **argv, const char *prefix)\n+int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n {\n \tconst char **pathspec = NULL;\n \n@@ -252,7 +252,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)\n \t}\n \n \treturn run_add_interactive(NULL,\n-\t\t\t\t   patch_interactive ? \"--patch\" : NULL,\n+\t\t\t\t   patch ? \"--patch\" : NULL,\n \t\t\t\t   pathspec);\n }\n \n@@ -377,7 +377,7 @@ int cmd_add(int argc, const char **argv, const char *prefix)\n \tif (patch_interactive)\n \t\tadd_interactive = 1;\n \tif (add_interactive)\n-\t\texit(interactive_add(argc - 1, argv + 1, prefix));\n+\t\texit(interactive_add(argc - 1, argv + 1, prefix, patch_interactive));\n \n \tif (edit_interactive)\n \t\treturn(edit_patch(argc, argv, prefix));\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 7707af8..008c1ec 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -83,7 +83,7 @@ static const char *template_file;\n static const char *author_message, *author_message_buffer;\n static char *edit_message, *use_message;\n static char *fixup_message, *squash_message;\n-static int all, edit_flag, also, interactive, only, amend, signoff;\n+static int all, edit_flag, also, interactive, patch_interactive, only, amend, signoff;\n static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship;\n static int no_post_rewrite, allow_empty_message;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg;\n@@ -152,6 +152,7 @@ static struct option builtin_commit_options[] = {\n \tOPT_BOOLEAN('a', \"all\", &all, \"commit all changed files\"),\n \tOPT_BOOLEAN('i', \"include\", &also, \"add specified files to index for commit\"),\n \tOPT_BOOLEAN(0, \"interactive\", &interactive, \"interactively add files\"),\n+\tOPT_BOOLEAN('p', \"patch\", &patch_interactive, \"interactively add changes\"),\n \tOPT_BOOLEAN('o', \"only\", &only, \"commit only specified files\"),\n \tOPT_BOOLEAN('n', \"no-verify\", &no_verify, \"bypass pre-commit hook\"),\n \tOPT_BOOLEAN(0, \"dry-run\", &dry_run, \"show what would be committed\"),\n@@ -360,7 +361,7 @@ static char *prepare_index(int argc, const char **argv, const char *prefix, int\n \t\told_index_env = getenv(INDEX_ENVIRONMENT);\n \t\tsetenv(INDEX_ENVIRONMENT, index_lock.filename, 1);\n \n-\t\tif (interactive_add(argc, argv, prefix) != 0)\n+\t\tif (interactive_add(argc, argv, prefix, patch_interactive) != 0)\n \t\t\tdie(_(\"interactive add failed\"));\n \n \t\tif (old_index_env && *old_index_env)\n@@ -1061,8 +1062,11 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \t\tauthor_message_buffer = read_commit_message(author_message);\n \t}\n \n+\tif (patch_interactive)\n+\t\tinteractive = 1;\n+\n \tif (!!also + !!only + !!all + !!interactive > 1)\n-\t\tdie(_(\"Only one of --include/--only/--all/--interactive can be used.\"));\n+\t\tdie(_(\"Only one of --include/--only/--all/--interactive/--patch can be used.\"));\n \tif (argc == 0 && (also || (only && !amend)))\n \t\tdie(_(\"No paths with --include/--only does not make sense.\"));\n \tif (argc == 0 && only && amend)\ndiff --git a/commit.h b/commit.h\nindex b3c3bb7..43940e2 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -161,7 +161,7 @@ extern struct commit_list *get_shallow_commits(struct object_array *heads,\n int is_descendant_of(struct commit *, struct commit_list *);\n int in_merge_bases(struct commit *, struct commit **, int);\n \n-extern int interactive_add(int argc, const char **argv, const char *prefix);\n+extern int interactive_add(int argc, const char **argv, const char *prefix, int patch);\n extern int run_add_interactive(const char *revision, const char *patch_mode,\n \t\t\t       const char **pathspec);\n \n-- \n1.7.5.188.g4817\n"},{"id":"167329","messageId":"1304791144-3374-1-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"BANLkTi=Y5o=KP1LnkKqGq31Sqfn-ZZCGNA@mail.gmail.com","subject":"[PATCH] Add commit to list of config.singlekey commands","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-07T17:59:04Z","receivedAt":"2011-05-07T17:59:04Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n Documentation/config.txt |    7 ++++---\n 1 files changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/config.txt b/Documentation/config.txt\nindex 285c7f7..1a060ec 100644\n--- a/Documentation/config.txt\n+++ b/Documentation/config.txt\n@@ -1310,9 +1310,10 @@ interactive.singlekey::\n \tIn interactive commands, allow the user to provide one-letter\n \tinput with a single key (i.e., without hitting enter).\n \tCurrently this is used by the `\\--patch` mode of\n-\tlinkgit:git-add[1], linkgit:git-reset[1], linkgit:git-stash[1] and\n-\tlinkgit:git-checkout[1]. Note that this setting is silently\n-\tignored if portable keystroke input is not available.\n+\tlinkgit:git-add[1], linkgit:git-checkout[1], linkgit:git-commit[1],\n+\tlinkgit:git-reset[1], and linkgit:git-stash[1]. Note that this\n+\tsetting is silently ignored if portable keystroke input\n+\tis not available.\n \n log.date::\n \tSet the default date-time mode for the 'log' command.\n-- \n1.7.5.188.g4817\n"},{"id":"167441","messageId":"7vliyhro0z.fsf@alter.siamese.dyndns.org","threadId":"27289","inReplyTo":"1304791087-2965-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v4 3/3] Add support for -p/--patch to git-commit","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-08T19:27:24Z","receivedAt":"2011-05-08T19:27:24Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Conrad Irwin <conrad.irwin@gmail.com> writes:\n\n> diff --git a/builtin/add.c b/builtin/add.c\n> index d39a6ab..f02524b 100644\n> --- a/builtin/add.c\n> +++ b/builtin/add.c\n> @@ -241,7 +241,7 @@ int run_add_interactive(const char *revision, const char *patch_mode,\n>  \treturn status;\n>  }\n>  \n> -int interactive_add(int argc, const char **argv, const char *prefix)\n> +int interactive_add(int argc, const char **argv, const char *prefix, int patch)\n>  {\n>  \tconst char **pathspec = NULL;\n>  \n> @@ -252,7 +252,7 @@ int interactive_add(int argc, const char **argv, const char *prefix)\n>  \t}\n>  \n>  \treturn run_add_interactive(NULL,\n> -\t\t\t\t   patch_interactive ? \"--patch\" : NULL,\n> +\t\t\t\t   patch ? \"--patch\" : NULL,\n>  \t\t\t\t   pathspec);\n>  }\n\nThis removes one reason for patch_interactive to be a file level global\nvariable. Perhaps we should plan to move builtin_add_options[] and the\nfile level global variables in builtin/add.c to cmd_add() and turn them\ninto on-stack variables.\n\nIt obviously is a totally separate topic, though.\n"},{"id":"167501","messageId":"20110509144451.GA11362@sigill.intra.peff.net","threadId":"27289","inReplyTo":"1304748001-17982-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-09T14:44:51Z","receivedAt":"2011-05-09T14:44:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 06, 2011 at 10:59:58PM -0700, conrad.irwin@gmail.com wrote:\n\n> I've rebased my support for git commit -p onto the current master\n> branch. I've posted it to the list twice before [1][2].\n\nThanks for reposting. I had been meaning to look at this again, but\nhadn't gotten around to it. So thanks for being persistent. :)\n\n>   Use a temporary index for git commit --interactive\n\nI think the intent of this one is good. In reviewing your initial\nseries, I had wondered about consistency with respect to the atomicity\nof \"git commit -p\" versus \"git add -p\" (i.e., what state is the index\nleft in when you abort). But reading through the discussion again, I\nthink we should worry more about consistency between \"git commit -i\" and\n\"git commit --interactive\". That is, both should produce no changes to\nthe index when the commit is aborted. So I think your patch is a step in\nthe right direction.\n\nThat still leaves an inconsistency in \"git add -p\" versus \"git commit\n-p\" (e.g., if you abort \"git add -p\" with \"^C\"). But if we care, the\nright solution is probably to make \"git add -p\" atomic. That can be a\nseparate topic, though, and I'm not sure anyone really cares enough to\nwork on it.\n\nI have one final question. If I do abort a commit, is there any way to\nrecover the state that was in the temporary index? That is, if I abort\n\"git commit -i\" by using an empty commit message, it is easy enough to\nuse shell history to repeat the command (possibly with a different set\nof files). But if I spend some time selecting (and possibly editing)\nhunks, and then decide to abort the commit, is there any way to recover\nthe intermediate index state?\n\n>From my reading of the code, it looks like \"no\". We will rollback the\nlockfile which contains the new index when aborting the commit.\n\nI'm not sure if it is worth caring about. If you are really interested\nin index state, you are probably better off using \"git add -p\" and \"git\ncommit\" separately. And even if we kept the index file around, it\nrequires a fairly savvy plumbing user to be able to pick changes out of\nit.\n\n>   Allow git commit --interactive with paths\n\nHmm. Test t7501.8 explicitly tests that this isn't allowed. But the test\nis poorly written, and falsely returns success even with your patch.\n\nThe original test should have looked like this:\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 7f7f7c7..8090b3c 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -45,7 +45,8 @@ test_expect_success \\\n test_expect_success PERL \\\n \t\"using paths with --interactive\" \\\n \t\"echo bong-o-bong >file &&\n-\t! (echo 7 | git commit -m foo --interactive file)\"\n+\t! ({ echo 2; echo 1; echo; echo 7; } |\n+\tgit commit -m foo --interactive file)\"\n \n test_expect_success \\\n \t\"using invalid commit with -C\" \\\n\nwhich does properly fail with your change. Your commit should tweak that\ntest (speaking of which, it would be nice for patch 1 to have a test,\ntoo).\n\nOther than that, the code in all 3 looks fine to me.\n\n-Peff\n"},{"id":"167512","messageId":"7vei47q0i6.fsf@alter.siamese.dyndns.org","threadId":"27289","inReplyTo":"20110509144451.GA11362@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T16:53:05Z","receivedAt":"2011-05-09T16:53:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I'm not sure if it is worth caring about. If you are really interested\n> in index state, you are probably better off using \"git add -p\" and \"git\n> commit\" separately.\n\nI agree with this. I do not foresee myself using \"commit -p\" ever for this\nexact reason. I however didn't see anything wrong in the series and I do\nnot see any reason to reject it, either. It's just another long rope other\npeople can use to tangle their neck with ;-).\n\n> Hmm. Test t7501.8 explicitly tests that this isn't allowed. But the test\n> is poorly written, and falsely returns success even with your patch.\n\nThanks. Let me see if I can simply amend what I queued already ;-)\n"},{"id":"167538","messageId":"20110509220806.GC3719@sigill.intra.peff.net","threadId":"27289","inReplyTo":"7vei47q0i6.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-09T22:08:06Z","receivedAt":"2011-05-09T22:08:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 09, 2011 at 09:53:05AM -0700, Junio C Hamano wrote:\n\n> I agree with this. I do not foresee myself using \"commit -p\" ever for this\n> exact reason. I however didn't see anything wrong in the series and I do\n> not see any reason to reject it, either. It's just another long rope other\n> people can use to tangle their neck with ;-).\n\nHeh. Maybe your title should be \"Git Hangman\". :)\n\n> > Hmm. Test t7501.8 explicitly tests that this isn't allowed. But the test\n> > is poorly written, and falsely returns success even with your patch.\n> \n> Thanks. Let me see if I can simply amend what I queued already ;-)\n\nIt's unfortunately not quite as simple as having that test succeed, as\nit changes state that breaks later tests. I didn't investigate deeply,\nthough.\n\n-Peff\n"},{"id":"167559","messageId":"7vwrhzmnxf.fsf@alter.siamese.dyndns.org","threadId":"27289","inReplyTo":"20110509220806.GC3719@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T23:53:00Z","receivedAt":"2011-05-09T23:53:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> It's unfortunately not quite as simple as having that test succeed, as\n> it changes state that breaks later tests. I didn't investigate deeply,\n> though.\n\nYeah, that test that hardcodes the exact commit sequence is disgusting.\nIn the meantime...\n\n-- >8 --\nFrom: Jeff King <peff@peff.net>\nSubject: [PATCH] t7501.8: feed a meaningful command\n\nThe command expects \"git commit --interactive <path>\" to fail because you\ncannot (yet) limit \"commit --interactive\" with a pathspec, but even if the\ncommand allowed to take <path>, the test would have failed as saying just\n7:quit would leave the index the same as the current commit, leading to an\nattempt to create an empty commit that would fail without --allow-empty.\n\nSigned-off-by: Jeff King <peff@peff.net>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n t/t7501-commit.sh |   10 ++++++----\n 1 files changed, 6 insertions(+), 4 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex a76c474..3d2b14d 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -41,10 +41,12 @@ test_expect_success \\\n \t\"echo King of the bongo >file &&\n \ttest_must_fail git commit -m foo -a file\"\n \n-test_expect_success PERL \\\n-\t\"using paths with --interactive\" \\\n-\t\"echo bong-o-bong >file &&\n-\t! (echo 7 | git commit -m foo --interactive file)\"\n+test_expect_success PERL 'cannot use paths with --interactive' '\n+\techo bong-o-bong >file &&\n+\t# 2: update, 1:st path, that is all, 7: quit\n+\t( echo 2; echo 1; echo; echo 7 ) |\n+\ttest_must_fail git commit -m foo --interactive file\n+'\n \n test_expect_success \\\n \t\"using invalid commit with -C\" \\\n-- \n1.7.5.1.290.g1b565\n"},{"id":"167560","messageId":"7vsjsnmnrv.fsf@alter.siamese.dyndns.org","threadId":"27289","inReplyTo":"7vwrhzmnxf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-05-09T23:56:20Z","receivedAt":"2011-05-09T23:56:20Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> It's unfortunately not quite as simple as having that test succeed, as\n>> it changes state that breaks later tests. I didn't investigate deeply,\n>> though.\n>\n> Yeah, that test that hardcodes the exact commit sequence is disgusting.\n> In the meantime...\n>\n> -- >8 --\n> From: Jeff King <peff@peff.net>\n> Subject: [PATCH] t7501.8: feed a meaningful command\n\nAnd then on top of that, Conrad's \"allow commit --interactive <path>\"\nwould come, with this squashed in.  The last \"reset HEAD^\" is nasty but I\ndon't have enough energy to fix 12ace0b (Add test case for basic commit\nfunctionality., 2007-07-31) today.\n\n t/t7501-commit.sh |    5 +++--\n 1 files changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 3d2b14d..c2fd116 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -41,11 +41,12 @@ test_expect_success \\\n \t\"echo King of the bongo >file &&\n \ttest_must_fail git commit -m foo -a file\"\n \n-test_expect_success PERL 'cannot use paths with --interactive' '\n+test_expect_success PERL 'can use paths with --interactive' '\n \techo bong-o-bong >file &&\n \t# 2: update, 1:st path, that is all, 7: quit\n \t( echo 2; echo 1; echo; echo 7 ) |\n-\ttest_must_fail git commit -m foo --interactive file\n+\tgit commit -m foo --interactive file &&\n+\tgit reset --hard HEAD^\n '\n \n test_expect_success \\\n"},{"id":"167582","messageId":"BANLkTikwjZkzMxksBsVTFRYdhE3L6ZQM0A@mail.gmail.com","threadId":"27289","inReplyTo":"20110509144451.GA11362@sigill.intra.peff.net","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-10T06:42:17Z","receivedAt":"2011-05-10T06:42:17Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"On Mon, May 9, 2011 at 7:44 AM, Jeff King <peff@peff.net> wrote:\n>\n> That still leaves an inconsistency in \"git add -p\" versus \"git commit\n> -p\" (e.g., if you abort \"git add -p\" with \"^C\"). But if we care, the\n> right solution is probably to make \"git add -p\" atomic. That can be a\n> separate topic, though, and I'm not sure anyone really cares enough to\n> work on it.\n\nI have pondered this problem too. \"git add -p\" is a particularly unintuitive,\nit inherits the update-the-index-after-every-complete-file semantics of\n\"git add --interactive\", but without ever making the file boundaries clear\nto the user. Depending on how much this bugs me, I might try to fix it\nin the future, but I wonder whether any users of \"git add --interactive\"\nare relying on the file based granularity.\n\n>\n> I have one final question. If I do abort a commit, is there any way to\n> recover the state that was in the temporary index? That is, if I abort\n> \"git commit -i\" by using an empty commit message, it is easy enough to\n> use shell history to repeat the command (possibly with a different set\n> of files). But if I spend some time selecting (and possibly editing)\n> hunks, and then decide to abort the commit, is there any way to recover\n> the intermediate index state?\n\nNot really. You could use your knowledge of git-commit to assume that the\ntree object with the most recent ctime is probably useful, but that only\nworks some of the time. The occasions on which I have wanted to abort\nthe process half-way, I've just created a temporary commit with what I\nhave so far. A \"git reset --soft\" at that point makes the whole process\nlong-hand for \"git add -p\".\n\n>\n>>   Allow git commit --interactive with paths\n>\n> Hmm. Test t7501.8 explicitly tests that this isn't allowed. But the test\n> is poorly written, and falsely returns success even with your patch.\n>\n\nWell spotted. Thank you Junio for fixing that test.\n\n> which does properly fail with your change. Your commit should tweak that\n> test (speaking of which, it would be nice for patch 1 to have a test,\n> too).\n\nI'll try to get my head around the tests :).\n\nConrad\n"},{"id":"167612","messageId":"1305054751-12327-1-git-send-email-conrad.irwin@gmail.com","threadId":"27289","inReplyTo":"BANLkTikwjZkzMxksBsVTFRYdhE3L6ZQM0A@mail.gmail.com","subject":"[PATCH] Test atomic git-commit --interactive","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-10T19:12:31Z","receivedAt":"2011-05-10T19:12:31Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"Signed-off-by: Conrad Irwin <conrad.irwin@gmail.com>\n---\n t/t7501-commit.sh |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex a76c474..2a4e453 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -130,6 +130,15 @@ test_expect_success PERL \\\n \t\"interactive add\" \\\n \t\"echo 7 | git commit --interactive | grep 'What now'\"\n \n+test_expect_success PERL \\\n+\t\"--interactive doesn't change index if editor aborts\" \\\n+\t\"EDITOR=false echo zoo >file && \\\n+\ttest_must_fail git diff --exit-code > diff1 && \\\n+\t(echo u ; echo '*' ; echo q) |\\\n+\ttest_must_fail git commit --interactive && \\\n+\tgit diff > diff2 && \\\n+\tgit diff --no-index diff1 diff2\"\n+\n test_expect_success \\\n \t\"showing committed revisions\" \\\n \t\"git rev-list HEAD >current\"\n-- \n1.7.5.188.g4817\n"},{"id":"167615","messageId":"20110510194334.GA14456@sigill.intra.peff.net","threadId":"27289","inReplyTo":"1305054751-12327-1-git-send-email-conrad.irwin@gmail.com","subject":"Re: [PATCH] Test atomic git-commit --interactive","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-10T19:43:34Z","receivedAt":"2011-05-10T19:43:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 10, 2011 at 12:12:31PM -0700, Conrad Irwin wrote:\n\n> +test_expect_success PERL \\\n> +\t\"--interactive doesn't change index if editor aborts\" \\\n> +\t\"EDITOR=false echo zoo >file && \\\n\nThis EDITOR bit does nothing, since it sets the value only for the\n\"echo\" command. The test happens to do what you want, though, since we\nset EDITOR to \":\" in test-lib.sh, and that is also a fine value for your\ntest (it's probably a better test, anyway; it simulates the user\naborting with an empty message instead of the editor crashing).\n\nEven though the implicit EDITOR works, I think it makes sense to be\nexplicit that it is something we are relying on. So we need to put it in\nfront of the \"git commit\" invocation, like:\n\n  EDITOR=: test_must_fail git commit --interactive\n\nUnfortunately, some shells do behave reasonably when setting single-shot\nenvironment variables with functions, so we are stuck with:\n\n  (EDITOR=: && export EDITOR &&\n   test_must_fail git commit --interactive)\n\n> +\ttest_must_fail git diff --exit-code > diff1 && \\\n\nYou can drop the backslash-continuation on lines that end with \"&&\" or\n\"|\", which makes things a bit more readable.\n\nYou do still need ones for:\n\n  test_expect_success \"description\" \\\n                      \"actual test\"\n\nThe usual style for our tests is:\n\n  test_expect_success 'description' '\n          actual test\n  '\n\nbut this particular script does not follow that, so it's probably more\nsensible to follow the surrounding style as you did.\n\n> +\t(echo u ; echo '*' ; echo q) |\\\n> +\ttest_must_fail git commit --interactive && \\\n> +\tgit diff > diff2 && \\\n\nMinor style nit: we usually write \">file\" without the space.\n\n> +\tgit diff --no-index diff1 diff2\"\n\nWhen comparing results with a simple diff, we generally use \"test_cmp\",\nso that unrelated tests do not rely on \"git diff --no-index\".\n\nSo the result should look like:\n\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 7f7f7c7..ccb0b95 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -131,6 +131,16 @@ test_expect_success PERL \\\n \t\"interactive add\" \\\n \t\"echo 7 | git commit --interactive | grep 'What now'\"\n \n+test_expect_success PERL \\\n+\t\"--interactive doesn't change index if editor aborts\" \\\n+\t\"echo zoo >file &&\n+\ttest_must_fail git diff --exit-code >diff1 &&\n+\t(echo u ; echo '*' ; echo q) |\n+\t(EDITOR=: && export EDITOR &&\n+\t test_must_fail git commit --interactive) &&\n+\tgit diff >diff2 &&\n+\ttest_cmp diff1 diff2\"\n+\n test_expect_success \\\n \t\"showing committed revisions\" \\\n \t\"git rev-list HEAD >current\"\n\n-Peff\n"},{"id":"167617","messageId":"20110510195646.GC14456@sigill.intra.peff.net","threadId":"27289","inReplyTo":"7vwrhzmnxf.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-10T19:56:47Z","receivedAt":"2011-05-10T19:56:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 09, 2011 at 04:53:00PM -0700, Junio C Hamano wrote:\n\n> -- >8 --\n> From: Jeff King <peff@peff.net>\n> Subject: [PATCH] t7501.8: feed a meaningful command\n> \n> The command expects \"git commit --interactive <path>\" to fail because you\n> cannot (yet) limit \"commit --interactive\" with a pathspec, but even if the\n> command allowed to take <path>, the test would have failed as saying just\n> 7:quit would leave the index the same as the current commit, leading to an\n> attempt to create an empty commit that would fail without --allow-empty.\n> \n> Signed-off-by: Jeff King <peff@peff.net>\n> Signed-off-by: Junio C Hamano <gitster@pobox.com>\n\nThanks. The commit message and SBO-forgery are fine by me. :)\n\n-Peff\n"},{"id":"167618","messageId":"20110510200146.GD14456@sigill.intra.peff.net","threadId":"27289","inReplyTo":"7vsjsnmnrv.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/3] Git commit --patch (again)","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2011-05-10T20:01:46Z","receivedAt":"2011-05-10T20:01:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, May 09, 2011 at 04:56:20PM -0700, Junio C Hamano wrote:\n\n> > Yeah, that test that hardcodes the exact commit sequence is disgusting.\n> > In the meantime...\n> >\n> > -- >8 --\n> > From: Jeff King <peff@peff.net>\n> > Subject: [PATCH] t7501.8: feed a meaningful command\n> \n> And then on top of that, Conrad's \"allow commit --interactive <path>\"\n> would come, with this squashed in.  The last \"reset HEAD^\" is nasty but I\n> don't have enough energy to fix 12ace0b (Add test case for basic commit\n> functionality., 2007-07-31) today.\n> \n>  t/t7501-commit.sh |    5 +++--\n>  1 files changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\n> index 3d2b14d..c2fd116 100755\n> --- a/t/t7501-commit.sh\n> +++ b/t/t7501-commit.sh\n> @@ -41,11 +41,12 @@ test_expect_success \\\n>  \t\"echo King of the bongo >file &&\n>  \ttest_must_fail git commit -m foo -a file\"\n>  \n> -test_expect_success PERL 'cannot use paths with --interactive' '\n> +test_expect_success PERL 'can use paths with --interactive' '\n>  \techo bong-o-bong >file &&\n>  \t# 2: update, 1:st path, that is all, 7: quit\n>  \t( echo 2; echo 1; echo; echo 7 ) |\n> -\ttest_must_fail git commit -m foo --interactive file\n> +\tgit commit -m foo --interactive file &&\n> +\tgit reset --hard HEAD^\n>  '\n\nYeah, that reset is a hack. Looking through the tests that come after,\nI don't think the extra commit is silently hurting any of them, so it's\nreally just the stupid hard-coded rev-list one.\n\nI looked at rewriting it into something like \"git log --format=%s\", but\nI couldn't come up with a commit message that reasonably argued \"this\ntests the same thing as the original\". Because I haven't really figured\nout what the original is supposed to be testing. That we made commits?\nThat we made a particular sequence of commits with those messages? With\nparticular trees?\n\nI feel like each test should be responsible for figuring out whether it\ncommitted or not (and AFAICT, they do). So I would be tempted to just\ndelete the rev-list test.\n\n-Peff\n"},{"id":"167628","messageId":"BANLkTim-LxUh=rVLD=vevvpH7KsG=3crtQ@mail.gmail.com","threadId":"27289","inReplyTo":"20110510194334.GA14456@sigill.intra.peff.net","subject":"Re: [PATCH] Test atomic git-commit --interactive","fromName":"Conrad Irwin","fromEmail":"conrad.irwin@gmail.com","sentAt":"2011-05-10T21:01:48Z","receivedAt":"2011-05-10T21:01:48Z","isPatch":true,"sender":{"key":"conrad.irwin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/94272?v=4"},"body":"On Tue, May 10, 2011 at 12:43 PM, Jeff King <peff@peff.net> wrote:\n> On Tue, May 10, 2011 at 12:12:31PM -0700, Conrad Irwin wrote:\n>\n>> +test_expect_success PERL \\\n>> +     \"--interactive doesn't change index if editor aborts\" \\\n>> +     \"EDITOR=false echo zoo >file && \\\n>\n> This EDITOR bit does nothing, since it sets the value only for the\n> \"echo\" command. The test happens to do what you want, though, since we\n> set EDITOR to \":\" in test-lib.sh, and that is also a fine value for your\n> test (it's probably a better test, anyway; it simulates the user\n> aborting with an empty message instead of the editor crashing).\n\n>  (EDITOR=: && export EDITOR &&\n>   test_must_fail git commit --interactive)\n\nThat makes sense, I was unaware of the : command, thank you.\n\n> You can drop the backslash-continuation on lines that end with \"&&\" or\n> \"|\", which makes things a bit more readable.\n\nGood to know.\n\n> So the result should look like:\n\nAwesome, thanks.\n\nConrad\n"}]}