{"thread":{"id":"42883","subject":"[PATCH v1 0/6] Porcelain Status V2","startedAt":"2016-07-19T22:13:33Z","lastAt":"2016-07-22T17:01:18Z","messageCount":41,"participants":["Jeff Hostetler","Johannes Schindelin","Jakub Narębski","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":6},"messages":[{"id":"291772","messageId":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":null,"subject":"[PATCH v1 0/6] Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:52Z","receivedAt":"2016-07-19T22:13:33Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"This patch series adds porcelain V2 format to status.\nThis provides detailed information about file changes\nand about the current branch.\n\nThe new output is accessed via:\n git status --porcelain=2 [--branch]\n\nAn earlier draft of this work was submitted under the\n\"Add very verbose porcelain output to status\" title.\nThis new version addresses the concerns about using\n(abusing) \"-vv\" and simplifies the some of the formatting.\n\nThis version does not include the state header from my\nfirst draft. I agree that if guarded by a \"--state\"\nargument, that output could be added to both formats.\n\n\nJeff Hostetler (6):\n Allow --porcelain[=] in status and commit commands\n Status and checkout unit tests for --porcelain[=]\n Per-file output for Porcelain Status V2\n Expanded branch header for Porcelain Status V2\n Add porcelain V2 documentation to status manpage\n Unit tests for V2 porcelain status\n\n Documentation/git-commit.txt | 2 +-\n Documentation/git-status.txt | 69 +++++-\n builtin/commit.c | 54 +++--\n t/t7060-wtstatus.sh | 21 ++\n t/t7064-wtstatus-pv2.sh | 461 ++++++++++++++++++++++++++++++++++++++++\n t/t7501-commit.sh | 23 ++\n wt-status.c | 487 ++++++++++++++++++++++++++++++++++++++++++-\n wt-status.h | 25 +++\n 8 files changed, 1122 insertions(+), 20 deletions(-)\n create mode 100755 t/t7064-wtstatus-pv2.sh\n\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291773","messageId":"1468966258-11191-2-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:53Z","receivedAt":"2016-07-19T22:13:51Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"Update the --porcelain argument to take an optional\nversion number. This will allow us to define new\nporcelain formats in the future.\n\nThis default to 1 and represents the existing porcelain\nformat.\n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-commit.txt | 2 +-\n Documentation/git-status.txt | 7 +++++--\n builtin/commit.c | 40 +++++++++++++++++++++++++++-------------\n wt-status.h | 10 ++++++++++\n 4 files changed, 43 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex b0a294d..0791573 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -104,7 +104,7 @@ OPTIONS\n --branch::\n \tShow the branch and tracking info even in short-format.\n \n---porcelain::\n+--porcelain[=]::\n \tWhen doing a dry-run, give the output in a porcelain-ready\n \tformat. See linkgit:git-status[1] for details. Implies\n \t`--dry-run`.\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex e1e8f57..de97729 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -32,11 +32,14 @@ OPTIONS\n --branch::\n \tShow the branch and tracking info even in short-format.\n \n---porcelain::\n+--porcelain[=]::\n \tGive the output in an easy-to-parse format for scripts.\n \tThis is similar to the short output, but will remain stable\n \tacross Git versions and regardless of user configuration. See\n \tbelow for details.\n++\n+The version parameter is used to specify the format version.\n+This is optional and defaults to the original version '1' format.\n \n --long::\n \tGive the output in the long-format. This is the default.\n@@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1].\n \n -z::\n \tTerminate entries with NUL, instead of LF. This implies\n-\tthe `--porcelain` output format if no other format is given.\n+\tthe `--porcelain=1` output format if no other format is given.\n \n --column[=]::\n --no-column::\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 1f6dbcd..892d7f7 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -142,14 +142,24 @@ static int show_ignored_in_status, have_option_m;\n static const char *only_include_assumed;\n static struct strbuf message = STRBUF_INIT;\n \n-static enum status_format {\n-\tSTATUS_FORMAT_NONE = 0,\n-\tSTATUS_FORMAT_LONG,\n-\tSTATUS_FORMAT_SHORT,\n-\tSTATUS_FORMAT_PORCELAIN,\n+static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n \n-\tSTATUS_FORMAT_UNSPECIFIED\n-} status_format = STATUS_FORMAT_UNSPECIFIED;\n+static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)\n+{\n+\tenum wt_status_format *value = (enum wt_status_format *)opt->value;\n+\tif (unset) {\n+\t\t*value = STATUS_FORMAT_UNSPECIFIED;\n+\t} else if (arg) {\n+\t\tint n = strtol(arg, NULL, 10);\n+\t\tif (n == 1)\n+\t\t\t*value = STATUS_FORMAT_PORCELAIN;\n+\t\telse\n+\t\t\tdie(\"unsupported porcelain version\");\n+\t} else {\n+\t\t*value = STATUS_FORMAT_PORCELAIN;\n+\t}\n+\treturn 0;\n+}\n \n static int opt_parse_m(const struct option *opt, const char *arg, int unset)\n {\n@@ -500,6 +510,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \ts->fp = fp;\n \ts->nowarn = nowarn;\n \ts->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;\n+\ts->status_format = status_format;\n \n \twt_status_collect(s);\n \n@@ -1099,7 +1110,7 @@ static const char *read_commit_message(const char *name)\n * is not in effect here.\n */\n static struct status_deferred_config {\n-\tenum status_format status_format;\n+\tenum wt_status_format status_format;\n \tint show_branch;\n } status_deferred_config = {\n \tSTATUS_FORMAT_UNSPECIFIED,\n@@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \t\t\t N_(\"show status concisely\"), STATUS_FORMAT_SHORT),\n \t\tOPT_BOOL('b', \"branch\", &s.show_branch,\n \t\t\t N_(\"show branch information\")),\n-\t\tOPT_SET_INT(0, \"porcelain\", &status_format,\n-\t\t\t N_(\"machine-readable output\"),\n-\t\t\t STATUS_FORMAT_PORCELAIN),\n+\t\t{ OPTION_CALLBACK, 0, \"porcelain\", &status_format,\n+\t\t N_(\"version\"), N_(\"machine-readable output\"),\n+\t\t PARSE_OPT_OPTARG, opt_parse_porcelain },\n \t\tOPT_SET_INT(0, \"long\", &status_format,\n \t\t\t N_(\"show status in long format (default)\"),\n \t\t\t STATUS_FORMAT_LONG),\n@@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n \ts.ignore_submodule_arg = ignore_submodule_arg;\n+\ts.status_format = status_format;\n+\n \twt_status_collect(&s);\n \n \tif (0 <= fd)\n@@ -1622,8 +1635,9 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_SET_INT(0, \"short\", &status_format, N_(\"show status concisely\"),\n \t\t\t STATUS_FORMAT_SHORT),\n \t\tOPT_BOOL(0, \"branch\", &s.show_branch, N_(\"show branch information\")),\n-\t\tOPT_SET_INT(0, \"porcelain\", &status_format,\n-\t\t\t N_(\"machine-readable output\"), STATUS_FORMAT_PORCELAIN),\n+\t\t{ OPTION_CALLBACK, 0, \"porcelain\", &status_format,\n+\t\t N_(\"version\"), N_(\"machine-readable output\"),\n+\t\t PARSE_OPT_OPTARG, opt_parse_porcelain },\n \t\tOPT_SET_INT(0, \"long\", &status_format,\n \t\t\t N_(\"show status in long format (default)\"),\n \t\t\t STATUS_FORMAT_LONG),\ndiff --git a/wt-status.h b/wt-status.h\nindex 2ca93f6..fc80341 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -43,6 +43,15 @@ struct wt_status_change_data {\n \tunsigned new_submodule_commits : 1;\n };\n \n+ enum wt_status_format {\n+\tSTATUS_FORMAT_NONE = 0,\n+\tSTATUS_FORMAT_LONG,\n+\tSTATUS_FORMAT_SHORT,\n+\tSTATUS_FORMAT_PORCELAIN,\n+\n+\tSTATUS_FORMAT_UNSPECIFIED\n+ };\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -66,6 +75,7 @@ struct wt_status {\n \tint show_branch;\n \tint hints;\n \n+\tenum wt_status_format status_format;\n \t/* These are computed during processing of the individual sections */\n \tint commitable;\n \tint workdir_dirty;\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291774","messageId":"1468966258-11191-3-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:54Z","receivedAt":"2016-07-19T22:13:54Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"Simple unit tests to validate the argument parsing.\n\nSigned-off-by: Jeff Hostetler \n---\n t/t7060-wtstatus.sh | 21 +++++++++++++++++++++\n t/t7501-commit.sh | 14 ++++++++++++++\n 2 files changed, 35 insertions(+)\n\ndiff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh\nindex 44bf1d8..a39b0e2 100755\n--- a/t/t7060-wtstatus.sh\n+++ b/t/t7060-wtstatus.sh\n@@ -228,4 +228,25 @@ test_expect_success 'status --branch with detached HEAD' '\n \ttest_i18ncmp expected actual\n '\n \n+## Duplicate the above test and verify --porcelain=1 has no effect.\n+test_expect_success 'status --branch with detached HEAD' '\n+\tgit reset --hard &&\n+\tgit checkout master^0 &&\n+\tgit status --branch --porcelain=1 >actual &&\n+\tcat >expected <<-EOF &&\n+\t## HEAD (no branch)\n+\t?? .gitconfig\n+\t?? actual\n+\t?? expect\n+\t?? expected\n+\t?? mdconflict/\n+\tEOF\n+\ttest_i18ncmp expected actual\n+'\n+\n+## Verify parser error on --porcelain argument.\n+test_expect_failure 'status --porcelain=bogus' '\n+\tgit status --porcelain=bogus\n+'\n+\n test_done\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex d84897a..1a74d75 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -73,6 +73,10 @@ test_expect_success '--porcelain fails with nothing to commit' '\n \ttest_must_fail git commit -m initial --porcelain\n '\n \n+test_expect_success '--porcelain=1 fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --porcelain=1\n+'\n+\n test_expect_success '--long fails with nothing to commit' '\n \ttest_must_fail git commit -m initial --long\n '\n@@ -97,6 +101,16 @@ test_expect_failure '--porcelain with stuff to commit returns ok' '\n \tgit commit -m next -a --porcelain\n '\n \n+test_expect_failure '--porcelain=1 with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --porcelain=1\n+'\n+\n+test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --porcelain=bogus\n+'\n+\n test_expect_success '--long with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --long\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291775","messageId":"1468966258-11191-4-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:55Z","receivedAt":"2016-07-19T22:13:56Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"This commit sets up version 2 porcelain status and\ndefines the format of detail lines. This includes\nthe usual XY and pathname fields. It adds the various\nfile modes and SHAs and the rename score. For regular\nentries these values reflect the head, index and\nworktree. For unmerged entries these values reflect\nthe stage 1, 2, and 3 values.\n\nSigned-off-by: Jeff Hostetler \n---\n builtin/commit.c | 9 ++\n wt-status.c | 398 ++++++++++++++++++++++++++++++++++++++++++++++++++++++-\n wt-status.h | 13 ++\n 3 files changed, 419 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 892d7f7..b5ec9b9 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un\n \t\tint n = strtol(arg, NULL, 10);\n \t\tif (n == 1)\n \t\t\t*value = STATUS_FORMAT_PORCELAIN;\n+\t\telse if (n == 2)\n+\t\t\t*value = STATUS_FORMAT_PORCELAIN_V2;\n \t\telse\n \t\t\tdie(\"unsupported porcelain version\");\n \t} else {\n@@ -521,6 +523,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \tcase STATUS_FORMAT_PORCELAIN:\n \t\twt_porcelain_print(s);\n \t\tbreak;\n+\tcase STATUS_FORMAT_PORCELAIN_V2:\n+\t\twt_porcelain_v2_print(s);\n+\t\tbreak;\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n \t\tbreak;\n@@ -1120,6 +1125,7 @@ static struct status_deferred_config {\n static void finalize_deferred_config(struct wt_status *s)\n {\n \tint use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN &&\n+\t\t\t\t status_format != STATUS_FORMAT_PORCELAIN_V2 &&\n \t\t\t\t !s->null_termination);\n \n \tif (s->null_termination) {\n@@ -1409,6 +1415,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tcase STATUS_FORMAT_PORCELAIN:\n \t\twt_porcelain_print(&s);\n \t\tbreak;\n+\tcase STATUS_FORMAT_PORCELAIN_V2:\n+\t\twt_porcelain_v2_print(&s);\n+\t\tbreak;\n \tcase STATUS_FORMAT_UNSPECIFIED:\n \t\tdie(\"BUG: finalize_deferred_config() should have been called\");\n \t\tbreak;\ndiff --git a/wt-status.c b/wt-status.c\nindex c19b52c..2d4829e 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -406,6 +406,89 @@ static void wt_status_print_change_data(struct wt_status *s,\n \tstrbuf_release(&twobuf);\n }\n \n+\n+/* Copy info for both sides of a head-vs-index change\n+ * into the Porcelain V2 data.\n+ */\n+static void porcelain_v2_updated_entry(\n+\tstruct wt_status_change_data *d,\n+\tstruct diff_filepair *p)\n+{\n+\tswitch (p->status) {\n+\tcase DIFF_STATUS_ADDED:\n+\t\td->porcelain_v2.mode_index = p->two->mode;\n+\t\thashcpy(d->porcelain_v2.sha1_index, p->two->sha1);\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_DELETED:\n+\t\td->porcelain_v2.mode_head = p->one->mode;\n+\t\thashcpy(d->porcelain_v2.sha1_head, p->one->sha1);\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_RENAMED:\n+\t\td->porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;\n+\tcase DIFF_STATUS_COPIED:\n+\tcase DIFF_STATUS_MODIFIED:\n+\tcase DIFF_STATUS_TYPE_CHANGED:\n+\tcase DIFF_STATUS_UNMERGED:\n+\t\td->porcelain_v2.mode_head = p->one->mode;\n+\t\td->porcelain_v2.mode_index = p->two->mode;\n+\t\thashcpy(d->porcelain_v2.sha1_head, p->one->sha1);\n+\t\thashcpy(d->porcelain_v2.sha1_index, p->two->sha1);\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_UNKNOWN:\n+\t\t/* This should never happen. */\n+\t\tbreak;\n+\t}\n+}\n+\n+/* Copy info for both sides of an index-vs-worktree change\n+ * into the very verbose porcelain data.\n+ */\n+static void porcelain_v2_changed_entry(\n+\tstruct wt_status_change_data *d,\n+\tconst struct diff_filepair *p)\n+{\n+\tswitch (p->status) {\n+\tcase DIFF_STATUS_ADDED:\n+\t\td->porcelain_v2.mode_worktree = p->two->mode;\n+\t\t/* don't bother with worktree sha, since it is almost always zero. */\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_DELETED:\n+\t\td->porcelain_v2.mode_index = p->one->mode;\n+\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_COPIED:\n+\tcase DIFF_STATUS_MODIFIED:\n+\tcase DIFF_STATUS_RENAMED:\n+\tcase DIFF_STATUS_TYPE_CHANGED:\n+\t\td->porcelain_v2.mode_index = p->one->mode;\n+\t\td->porcelain_v2.mode_worktree = p->two->mode;\n+\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n+\t\t/* don't bother with worktree sha, since it is almost always zero. */\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_UNKNOWN:\n+\t\t/* This should never happen. */\n+\t\tbreak;\n+\n+\tcase DIFF_STATUS_UNMERGED:\n+\t\t/* This should never happen. */\n+\t\tbreak;\n+\t}\n+}\n+\n+static void porcelain_v2_added_initial_entry(\n+\tstruct wt_status_change_data *d,\n+\tconst struct cache_entry *ce)\n+{\n+\td->porcelain_v2.mode_index = ce->ce_mode;\n+\thashcpy(d->porcelain_v2.sha1_index, ce->sha1);\n+}\n+\n static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\t\t\t\t struct diff_options *options,\n \t\t\t\t\t void *data)\n@@ -433,6 +516,9 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n \t\td->dirty_submodule = p->two->dirty_submodule;\n \t\tif (S_ISGITLINK(p->two->mode))\n \t\t\td->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);\n+\n+\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n+\t\t\tporcelain_v2_changed_entry(d, p);\n \t}\n }\n \n@@ -486,6 +572,9 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n \t\t\td->stagemask = unmerged_mask(p->two->path);\n \t\t\tbreak;\n \t\t}\n+\n+\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n+\t\t\tporcelain_v2_updated_entry(d, p);\n \t}\n }\n \n@@ -565,8 +654,12 @@ static void wt_status_collect_changes_initial(struct wt_status *s)\n \t\t\td->index_status = DIFF_STATUS_UNMERGED;\n \t\t\td->stagemask |= (1 << (ce_stage(ce) - 1));\n \t\t}\n-\t\telse\n+\t\telse {\n \t\t\td->index_status = DIFF_STATUS_ADDED;\n+\n+\t\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n+\t\t\t\tporcelain_v2_added_initial_entry(d, ce);\n+\t\t}\n \t}\n }\n \n@@ -1753,3 +1846,306 @@ void wt_porcelain_print(struct wt_status *s)\n \ts->no_gettext = 1;\n \twt_shortstatus_print(s);\n }\n+\n+/* Convert various submodule status values into a\n+ * string of characters in the buffer provided.\n+ */\n+static void wt_porcelain_v2_submodule_state(\n+\tstruct wt_status_change_data *d,\n+\tchar sub[5])\n+{\n+\tint k = 0;\n+\n+\tif (S_ISGITLINK(d->porcelain_v2.mode_head) ||\n+\t\tS_ISGITLINK(d->porcelain_v2.mode_index) ||\n+\t\tS_ISGITLINK(d->porcelain_v2.mode_worktree)) {\n+\t\t/* We have a submodule */\n+\t\tsub[k++] = 'S';\n+\n+\t\t/* Sub-flags for each type of dirt */\n+\t\tif (d->new_submodule_commits)\n+\t\t\tsub[k++] = 'C';\n+\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n+\t\t\tsub[k++] = 'M';\n+\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n+\t\t\tsub[k++] = 'U';\n+\t} else {\n+\t\t/* Not a submodule */\n+\t\tsub[k++] = 'N';\n+\t}\n+\n+\tsub[k] = 0;\n+}\n+\n+/* Various fix-up steps before we start printing an item.\n+ */\n+static void wt_porcelain_v2_fix_up_status(\n+\tstruct string_list_item *it,\n+\tstruct wt_status *s)\n+{\n+\tstruct wt_status_change_data *d = it->util;\n+\n+\tif (!d->index_status) {\n+\t\tif (d->worktree_status == DIFF_STATUS_MODIFIED ||\n+\t\t\td->worktree_status == DIFF_STATUS_DELETED) {\n+\t\t\t/* X=' ' Y=[MD]\n+\t\t\t * The item did not change in head-vs-index scan so the head\n+\t\t\t * column was never set. (The index column was set during the\n+\t\t\t * index-vs-worktree scan.)\n+\t\t\t * Force set the head column to make the output complete.\n+\t\t\t */\n+\t\t\td->porcelain_v2.mode_head = d->porcelain_v2.mode_index;\n+\t\t\thashcpy(d->porcelain_v2.sha1_head, d->porcelain_v2.sha1_index);\n+\t\t}\n+\t}\n+\n+\tif (!d->worktree_status) {\n+\t\tif (d->index_status == DIFF_STATUS_MODIFIED ||\n+\t\t\td->index_status == DIFF_STATUS_ADDED ||\n+\t\t\td->index_status == DIFF_STATUS_RENAMED ||\n+\t\t\td->index_status == DIFF_STATUS_COPIED) {\n+\t\t\t/* X=[MARC] Y=' '\n+\t\t\t * The item did not changed in the index-vs-worktree scan so\n+\t\t\t * the worktree column was never set.\n+\t\t\t * Force set the worktree mode to make the output complete.\n+\t\t\t */\n+\t\t\td->porcelain_v2.mode_worktree = d->porcelain_v2.mode_index;\n+\t\t}\n+\t}\n+}\n+\n+/*\n+ * Define a single format for tracked entries. This includes:\n+ * normal changes, rename changes, and unmerged changes.\n+ *\n+ * The meanings of modes_[abcd] and sha_[abc] depends on the\n+ * change type, but are always present.\n+ *\n+ * Path(s) are C-Quoted if necessary. Current path is ALWAYS\n+ * first. The rename source path is only present when necessary.\n+ * A single TAB separates them (because paths can contain spaces\n+ * and C-Quoting converts actual tabs in pathnames to a C escape\n+ * sequence).\n+ */\n+static void wt_porcelain_v2_print_tracked_entry(\n+\tFILE *fp,\n+\tchar x_staged,\n+\tchar y_unstaged,\n+\tconst char *submodule,\n+\tint mode_a,\n+\tint mode_b,\n+\tint mode_c,\n+\tint mode_d,\n+\tconst unsigned char sha_a[GIT_SHA1_RAWSZ],\n+\tconst unsigned char sha_b[GIT_SHA1_RAWSZ],\n+\tconst unsigned char sha_c[GIT_SHA1_RAWSZ],\n+\tint rename_score,\n+\tconst char *path_current,\n+\tconst char *path_rename_src,\n+\tint null_termination)\n+{\n+\tchar sep_char = null_termination ? '\\0' : '\\t';\n+\tchar eol_char = null_termination ? '\\0' : '\\n';\n+\n+\tif (path_rename_src)\n+\t\tfprintf(fp, \"%c%c %s %06o %06o %06o %06o %s %s %s R%d %s%c%s%c\",\n+\t\t\t\tx_staged, y_unstaged, submodule,\n+\t\t\t\tmode_a, mode_b, mode_c, mode_d,\n+\t\t\t\tsha1_to_hex(sha_a), sha1_to_hex(sha_b), sha1_to_hex(sha_c),\n+\t\t\t\trename_score,\n+\t\t\t\tpath_current, sep_char, path_rename_src,\n+\t\t\t\teol_char);\n+\telse\n+\t\tfprintf(fp, \"%c%c %s %06o %06o %06o %06o %s %s %s R%d %s%c\",\n+\t\t\t\tx_staged, y_unstaged, submodule,\n+\t\t\t\tmode_a, mode_b, mode_c, mode_d,\n+\t\t\t\tsha1_to_hex(sha_a), sha1_to_hex(sha_b), sha1_to_hex(sha_c),\n+\t\t\t\trename_score,\n+\t\t\t\tpath_current,\n+\t\t\t\teol_char);\n+}\n+\n+/*\n+ * Print porcelain V2 info for normal tracked entries.\n+ */\n+static void wt_porcelain_v2_print_normal_entry(\n+\tstruct string_list_item *it,\n+\tstruct wt_status *s)\n+{\n+\tstatic const unsigned char sha_zero[GIT_SHA1_RAWSZ] = {0};\n+\tstruct wt_status_change_data *d = it->util;\n+\tstruct strbuf buf_current = STRBUF_INIT;\n+\tstruct strbuf buf_rename_src = STRBUF_INIT;\n+\tconst char *path_current = NULL;\n+\tconst char *path_rename_src = NULL;\n+\tchar x_staged, y_unstaged;\n+\tchar submodule[5];\n+\n+\twt_porcelain_v2_fix_up_status(it, s);\n+\tx_staged = d->index_status ? d->index_status : '.';\n+\ty_unstaged = d->worktree_status ? d->worktree_status : '.';\n+\twt_porcelain_v2_submodule_state(d, submodule);\n+\n+\tif (s->null_termination) {\n+\t\tpath_current = it->string;\n+\t\tpath_rename_src = d->head_path;\n+\t} else {\n+\t\tpath_current = quote_path(it->string, s->prefix, &buf_current);\n+\t\tif (d->head_path)\n+\t\t\tpath_rename_src = quote_path(d->head_path, s->prefix, &buf_rename_src);\n+\t}\n+\n+\twt_porcelain_v2_print_tracked_entry(\n+\t\ts->fp,\n+\t\tx_staged, y_unstaged, submodule,\n+\t\td->porcelain_v2.mode_head,\n+\t\td->porcelain_v2.mode_index,\n+\t\td->porcelain_v2.mode_worktree,\n+\t\t0,\n+\t\td->porcelain_v2.sha1_head,\n+\t\td->porcelain_v2.sha1_index,\n+\t\tsha_zero,\n+\t\td->porcelain_v2.rename_score,\n+\t\tpath_current,\n+\t\tpath_rename_src,\n+\t\ts->null_termination);\n+\n+\tstrbuf_release(&buf_current);\n+\tstrbuf_release(&buf_rename_src);\n+}\n+\n+/*\n+ * Print very verbose porcelain status info for unmerged entries.\n+ */\n+static void wt_porcelain_v2_print_unmerged_entry(\n+\tstruct string_list_item *it,\n+\tstruct wt_status *s)\n+{\n+\tstruct wt_status_change_data *d = it->util;\n+\tconst struct cache_entry *ce;\n+\tstruct strbuf buf_current = STRBUF_INIT;\n+\tconst char *path_current = NULL;\n+\tint pos, stage;\n+\tstruct {\n+\t\tint mode;\n+\t\tunsigned char sha1[GIT_SHA1_RAWSZ];\n+\t} stages[3];\n+\tchar x_us = 'U', y_them = 'U';\n+\tchar submodule[5];\n+\n+\tswitch (d->stagemask) {\n+\tcase 1: x_us = 'D'; y_them = 'D'; break; /* both deleted */\n+\tcase 2: x_us = 'A'; y_them = 'U'; break; /* added by us */\n+\tcase 3: x_us = 'U'; y_them = 'D'; break; /* deleted by them */\n+\tcase 4: x_us = 'U'; y_them = 'A'; break; /* added by them */\n+\tcase 5: x_us = 'D'; y_them = 'U'; break; /* deleted by us */\n+\tcase 6: x_us = 'A'; y_them = 'A'; break; /* both added */\n+\tcase 7: x_us = 'U'; y_them = 'U'; break; /* both modified */\n+\t}\n+\n+\twt_porcelain_v2_submodule_state(d, submodule);\n+\n+\t/*\n+\t * Disregard the V2 {mode,sha} values for head and index\n+\t * that we computed from the diffs and lookup the actual\n+\t * stage data.\n+\t */\n+\tmemset(stages, 0, sizeof(stages));\n+\tpos = cache_name_pos(it->string, strlen(it->string));\n+\tassert(pos < 0);\n+\tpos = -pos-1;\n+\twhile (pos < active_nr) {\n+\t\tce = active_cache[pos++];\n+\t\tstage = ce_stage(ce);\n+\t\tif (strcmp(ce->name, it->string) || !stage)\n+\t\t\tbreak;\n+\t\tstages[stage - 1].mode = ce->ce_mode;\n+\t\thashcpy(stages[stage - 1].sha1, ce->sha1);\n+\t}\n+\n+\tif (s->null_termination)\n+\t\tpath_current = it->string;\n+\telse\n+\t\tpath_current = quote_path(it->string, s->prefix, &buf_current);\n+\n+\twt_porcelain_v2_print_tracked_entry(\n+\t\ts->fp, x_us, y_them, submodule,\n+\t\tstages[0].mode, /* stage 1 */\n+\t\tstages[1].mode, /* stage 2 */\n+\t\tstages[2].mode, /* stage 3 */\n+\t\td->porcelain_v2.mode_worktree,\n+\t\tstages[0].sha1, /* stage 1 */\n+\t\tstages[1].sha1, /* stage 2 */\n+\t\tstages[2].sha1, /* stage 3 */\n+\t\t0,\n+\t\tpath_current,\n+\t\tNULL,\n+\t\ts->null_termination);\n+\n+\tstrbuf_release(&buf_current);\n+}\n+\n+/*\n+ * Print porcelain V2 status info for untracked and ignored entries.\n+ */\n+static void wt_porcelain_v2_print_other(\n+\tstruct string_list_item *it,\n+\tstruct wt_status *s,\n+\tconst char *sign)\n+{\n+\tstruct strbuf buf = STRBUF_INIT;\n+\tconst char *path;\n+\tchar eol;\n+\n+\tif (s->null_termination) {\n+\t\tpath = it->string;\n+\t\teol = '\\0';\n+\t} else {\n+\t\tpath = quote_path(it->string, s->prefix, &buf);\n+\t\teol = '\\n';\n+\t}\n+\n+\tfprintf(s->fp, \"%s %s%c\", sign, path, eol);\n+\n+\tstrbuf_release(&buf);\n+}\n+\n+/* Print porcelain V2 status.\n+ *\n+ * []\n+ * []*\n+ * []*\n+ * []*\n+ *\n+ */\n+void wt_porcelain_v2_print(struct wt_status *s)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d;\n+\t\tstruct string_list_item *it;\n+\n+\t\tit = &(s->change.items[i]);\n+\t\td = it->util;\n+\n+\t\tif (d->stagemask)\n+\t\t\twt_porcelain_v2_print_unmerged_entry(it, s);\n+\t\telse\n+\t\t\twt_porcelain_v2_print_normal_entry(it, s);\n+\t}\n+\n+\tfor (i = 0; i < s->untracked.nr; i++) {\n+\t\tstruct string_list_item *it;\n+\n+\t\tit = &(s->untracked.items[i]);\n+\t\twt_porcelain_v2_print_other(it, s, \"??\");\n+\t}\n+\n+\tfor (i = 0; i < s->ignored.nr; i++) {\n+\t\tstruct string_list_item *it;\n+\n+\t\tit = &(s->ignored.items[i]);\n+\t\twt_porcelain_v2_print_other(it, s, \"!!\");\n+\t}\n+}\ndiff --git a/wt-status.h b/wt-status.h\nindex fc80341..c062e30 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -39,6 +39,17 @@ struct wt_status_change_data {\n \tint index_status;\n \tint stagemask;\n \tchar *head_path;\n+\n+\tstruct\n+\t{\n+\t\tint rename_score;\n+\t\tint mode_head;\n+\t\tint mode_index;\n+\t\tint mode_worktree;\n+\t\tunsigned char sha1_head[GIT_SHA1_RAWSZ];\n+\t\tunsigned char sha1_index[GIT_SHA1_RAWSZ];\n+\t} porcelain_v2;\n+\n \tunsigned dirty_submodule : 2;\n \tunsigned new_submodule_commits : 1;\n };\n@@ -48,6 +59,7 @@ struct wt_status_change_data {\n \tSTATUS_FORMAT_LONG,\n \tSTATUS_FORMAT_SHORT,\n \tSTATUS_FORMAT_PORCELAIN,\n+\tSTATUS_FORMAT_PORCELAIN_V2,\n \n \tSTATUS_FORMAT_UNSPECIFIED\n };\n@@ -119,6 +131,7 @@ int wt_status_check_bisect(const struct worktree *wt,\n \n void wt_shortstatus_print(struct wt_status *s);\n void wt_porcelain_print(struct wt_status *s);\n+void wt_porcelain_v2_print(struct wt_status *s);\n \n __attribute__((format (printf, 3, 4)))\n void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, ...);\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291776","messageId":"1468966258-11191-5-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:56Z","receivedAt":"2016-07-19T22:13:57Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"This commit expand porcelain status V2 to print\ndetailed information about the current branch.\nThis includes the SHA of the current commit, the\ncurrent branch name, the upstream branch name,\nand the ahead/behind counts.\n\nThis additional information is included when\nthe --branch argument is given.\n\nSigned-off-by: Jeff Hostetler \n---\n builtin/commit.c | 5 ++++\n wt-status.c | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n wt-status.h | 2 ++\n 3 files changed, 96 insertions(+)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex b5ec9b9..830f688 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -512,6 +512,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int\n \ts->fp = fp;\n \ts->nowarn = nowarn;\n \ts->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;\n+\tif (!s->is_initial)\n+\t\thashcpy(s->sha_commit, sha1);\n \ts->status_format = status_format;\n \n \twt_status_collect(s);\n@@ -1397,6 +1399,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n \tfd = hold_locked_index(&index_lock, 0);\n \n \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n+\tif (!s.is_initial)\n+\t\thashcpy(s.sha_commit, sha1);\n+\n \ts.ignore_submodule_arg = ignore_submodule_arg;\n \ts.status_format = status_format;\n \ndiff --git a/wt-status.c b/wt-status.c\nindex 2d4829e..9ba4da5 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1847,6 +1847,92 @@ void wt_porcelain_print(struct wt_status *s)\n \twt_shortstatus_print(s);\n }\n \n+/*\n+ * Print branch and tracking header for porcelain v2 output.\n+ * This is printed when the '--branch' parameter is given.\n+ *\n+ * \"## branch: [ [ + -]]\"\n+ *\n+ * ::= 40 character SHA1 of the current commit or\n+ * \"(initial)\" literal to indicate initialized\n+ * repo with no commits.\n+ *\n+ * ::= the current branch name or\n+ * \"(detached)\" literal when detached head.\n+ *\n+ * ::= the upstream branch name, when set.\n+ *\n+ * ::= integer ahead value, when upstream set\n+ * and commit is present.\n+ *\n+ * ::= integer behind value, when upstream set\n+ * and commit is present.\n+ *\n+ *\n+ * The end-of-line is defined by the -z flag.\n+ *\n+ * ::= NUL when -z,\n+ * LF when NOT -z.\n+ *\n+ */\n+static void wt_porcelain_v2_print_tracking(struct wt_status *s)\n+{\n+\tstruct branch *branch;\n+\tconst char *base;\n+\tconst char *branch_name;\n+\tstruct wt_status_state state;\n+\tint ab_info, nr_ahead, nr_behind;\n+\n+\tmemset(&state, 0, sizeof(state));\n+\twt_status_get_state(&state, s->branch && !strcmp(s->branch, \"HEAD\"));\n+\n+\tfprintf(s->fp, \"## branch:\");\n+\tfprintf(s->fp, \" %s\",\n+\t\t(s->is_initial ? \"(initial)\" : sha1_to_hex(s->sha_commit)));\n+\n+\tif (s->branch) {\n+\t\tif (!strcmp(s->branch, \"HEAD\")) {\n+\t\t\tfprintf(s->fp, \" %s\", \"(detached)\");\n+\t\t\tif (state.rebase_in_progress || state.rebase_interactive_in_progress) {\n+\t\t\t\tbranch_name = state.onto;\n+\t\t\t} else if (state.detached_from) {\n+\t\t\t\tbranch_name = state.detached_from;\n+\t\t\t} else {\n+\t\t\t\tbranch_name = \"\";\n+\t\t\t}\n+\t\t} else {\n+\t\t\tbranch_name = NULL;\n+\t\t\tskip_prefix(s->branch, \"refs/heads/\", &branch_name);\n+\t\t\tfprintf(s->fp, \" %s\", branch_name);\n+\t\t}\n+\n+\t\t/* Lookup stats on the upstream tracking branch, if set. */\n+\t\tbranch = branch_get(branch_name);\n+\t\tbase = NULL;\n+\t\tab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, &base) == 0);\n+\t\tif (base) {\n+\t\t\tbase = shorten_unambiguous_ref(base, 0);\n+\t\t\tfprintf(s->fp, \" %s\", base);\n+\t\t\tfree((char *)base);\n+\n+\t\t\tif (ab_info)\n+\t\t\t\tfprintf(s->fp, \" +%d -%d\", nr_ahead, nr_behind);\n+\t\t}\n+\t} else {\n+\t\t/*\n+\t\t * TODO All of various print routines allow for s->branch to be null.\n+\t\t * TODO When can this happen and what should we report here?\n+\t\t */\n+\t\tfprintf(s->fp, \" %s\", \"(unknown)\");\n+\t}\n+\n+\tfprintf(s->fp, \"%c\", (s->null_termination ? '\\0' : '\\n'));\n+\n+\tfree(state.branch);\n+\tfree(state.onto);\n+\tfree(state.detached_from);\n+}\n+\n /* Convert various submodule status values into a\n * string of characters in the buffer provided.\n */\n@@ -2122,6 +2208,9 @@ void wt_porcelain_v2_print(struct wt_status *s)\n {\n \tint i;\n \n+\tif (s->show_branch)\n+\t\twt_porcelain_v2_print_tracking(s);\n+\n \tfor (i = 0; i < s->change.nr; i++) {\n \t\tstruct wt_status_change_data *d;\n \t\tstruct string_list_item *it;\ndiff --git a/wt-status.h b/wt-status.h\nindex c062e30..168556f 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -88,6 +88,8 @@ struct wt_status {\n \tint hints;\n \n \tenum wt_status_format status_format;\n+\tunsigned char sha_commit[GIT_SHA1_RAWSZ]; /* Commit SHA (when not Initial) */\n+\n \t/* These are computed during processing of the individual sections */\n \tint commitable;\n \tint workdir_dirty;\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291777","messageId":"1468966258-11191-7-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 6/6] Unit tests for V2 porcelain status","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:58Z","receivedAt":"2016-07-19T22:13:59Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"This commit contains unit tests to exercise\nthe V2 porcelain status format.\n\nSigned-off-by: Jeff Hostetler \n---\n t/t7064-wtstatus-pv2.sh | 461 ++++++++++++++++++++++++++++++++++++++++++++++++\n t/t7501-commit.sh | 9 +\n 2 files changed, 470 insertions(+)\n create mode 100755 t/t7064-wtstatus-pv2.sh\n\ndiff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh\nnew file mode 100755\nindex 0000000..1f4252a\n--- /dev/null\n+++ b/t/t7064-wtstatus-pv2.sh\n@@ -0,0 +1,461 @@\n+#!/bin/sh\n+\n+test_description='git status --porcelain=2\n+\n+This test exercises porcelain V2 output for git status.'\n+\n+\n+. ./test-lib.sh\n+\n+test_expect_success setup '\n+\ttest_tick &&\n+\tgit config --local core.autocrlf false &&\n+\techo x >file_x &&\n+\techo y >file_y &&\n+\techo z >file_z &&\n+\tmkdir dir1 &&\n+\techo a >dir1/file_a &&\n+\techo b >dir1/file_b\n+'\n+\n+\n+##################################################################\n+## Confirm VVP output prior to initial commit.\n+##################################################################\n+\n+test_expect_success pre_initial_commit_0 '\n+\tprintf \"## branch: (initial) master\\n\" >expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? dir1/\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\tprintf \"?? file_x\\n\" >>expected &&\n+\tprintf \"?? file_y\\n\" >>expected &&\n+\tprintf \"?? file_z\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=normal >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success pre_initial_commit_1 '\n+\tgit add file_x file_y file_z dir1 &&\n+\tSHA_A=`git hash-object -t blob -- dir1/file_a` &&\n+\tSHA_B=`git hash-object -t blob -- dir1/file_b` &&\n+\tSHA_X=`git hash-object -t blob -- file_x` &&\n+\tSHA_Y=`git hash-object -t blob -- file_y` &&\n+\tSHA_Z=`git hash-object -t blob -- file_z` &&\n+\tSHA_ZERO=0000000000000000000000000000000000000000 &&\n+\tprintf \"## branch: (initial) master\\n\" >expected &&\n+\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $SHA_A $SHA_ZERO R0 dir1/file_a\\n\" >>expected &&\n+\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $SHA_B $SHA_ZERO R0 dir1/file_b\\n\" >>expected &&\n+\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $SHA_X $SHA_ZERO R0 file_x\\n\" >>expected &&\n+\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $SHA_Y $SHA_ZERO R0 file_y\\n\" >>expected &&\n+\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $SHA_Z $SHA_ZERO R0 file_z\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+##################################################################\n+## Create first commit. Confirm commit sha in new track header.\n+## Then make some changes on top of it.\n+##################################################################\n+\n+test_expect_success initial_commit_0 '\n+\tgit commit -m initial &&\n+\tH0=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H0 master\\n\" >expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success initial_commit_1 '\n+\techo x >>file_x &&\n+\tSHA_X1=`git hash-object -t blob -- file_x` &&\n+\trm file_z &&\n+\tH0=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H0 master\\n\" >expected &&\n+\tprintf \".M N 100644 100644 100644 000000 $SHA_X $SHA_X $SHA_ZERO R0 file_x\\n\" >>expected &&\n+\tprintf \".D N 100644 100644 000000 000000 $SHA_Z $SHA_Z $SHA_ZERO R0 file_z\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success initial_commit_2 '\n+\tgit add file_x &&\n+\tgit rm file_z &&\n+\tH0=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H0 master\\n\" >expected &&\n+\tprintf \"M. N 100644 100644 100644 000000 $SHA_X $SHA_X1 $SHA_ZERO R0 file_x\\n\" >>expected &&\n+\tprintf \"D. N 100644 000000 000000 000000 $SHA_Z $SHA_ZERO $SHA_ZERO R0 file_z\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success initial_commit_3 '\n+\tgit mv file_y renamed_y &&\n+\tH0=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H0 master\\n\" >expected &&\n+\tprintf \"M. N 100644 100644 100644 000000 $SHA_X $SHA_X1 $SHA_ZERO R0 file_x\\n\" >>expected &&\n+\tprintf \"D. N 100644 000000 000000 000000 $SHA_Z $SHA_ZERO $SHA_ZERO R0 file_z\\n\" >>expected &&\n+\tprintf \"R. N 100644 100644 100644 000000 $SHA_Y $SHA_Y $SHA_ZERO R100 renamed_y\\tfile_y\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+##################################################################\n+## Create second commit.\n+##################################################################\n+\n+test_expect_success second_commit_0 '\n+\tgit commit -m second &&\n+\tH1=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H1 master\\n\" >expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+##################################################################\n+## Ignore a file\n+##################################################################\n+\n+test_expect_success ignore_file_0 '\n+\techo x.ign >.gitignore &&\n+\techo \"ignore me\" >x.ign &&\n+\tH1=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $H1 master\\n\" >expected &&\n+\tprintf \"?? .gitignore\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\tprintf \"!! x.ign\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\trm x.ign &&\n+\trm .gitignore &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+##################################################################\n+## Create some conflicts.\n+##################################################################\n+\n+test_expect_success conflict_AA '\n+\tgit branch AA_A master &&\n+\tgit checkout AA_A &&\n+\techo \"Branch AA_A\" >conflict.txt &&\n+\tSHA_AA_A=`git hash-object -t blob -- conflict.txt` &&\n+\tgit add conflict.txt &&\n+\tgit commit -m \"branch aa_a\" &&\n+\n+\tgit branch AA_B master &&\n+\tgit checkout AA_B &&\n+\techo \"Branch AA_B\" >conflict.txt &&\n+\tSHA_AA_B=`git hash-object -t blob -- conflict.txt` &&\n+\tgit add conflict.txt &&\n+\tgit commit -m \"branch aa_b\" &&\n+\n+\tgit branch AA_M AA_B &&\n+\tgit checkout AA_M &&\n+\ttest_must_fail git merge AA_A &&\n+\n+\tHM=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $HM AA_M\\n\" >expected &&\n+\tprintf \"AA N 000000 100644 100644 100644 $SHA_ZERO $SHA_AA_B $SHA_AA_A R0 conflict.txt\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\tgit reset --hard &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+test_expect_success conflict_UU '\n+\tgit branch UU_ANC master &&\n+\tgit checkout UU_ANC &&\n+\techo \"Ancestor\" >conflict.txt &&\n+\tSHA_UU_ANC=`git hash-object -t blob -- conflict.txt` &&\n+\tgit add conflict.txt &&\n+\tgit commit -m \"UU_ANC\" &&\n+\n+\tgit branch UU_A UU_ANC &&\n+\tgit checkout UU_A &&\n+\techo \"Branch UU_A\" >conflict.txt &&\n+\tSHA_UU_A=`git hash-object -t blob -- conflict.txt` &&\n+\tgit add conflict.txt &&\n+\tgit commit -m \"branch uu_a\" &&\n+\n+\tgit branch UU_B UU_ANC &&\n+\tgit checkout UU_B &&\n+\techo \"Branch UU_B\" >conflict.txt &&\n+\tSHA_UU_B=`git hash-object -t blob -- conflict.txt` &&\n+\tgit add conflict.txt &&\n+\tgit commit -m \"branch uu_b\" &&\n+\n+\tgit branch UU_M UU_B &&\n+\tgit checkout UU_M &&\n+\ttest_must_fail git merge UU_A &&\n+\n+\tHM=`git rev-parse HEAD` &&\n+\tprintf \"## branch: $HM UU_M\\n\" >expected &&\n+\tprintf \"UU N 100644 100644 100644 100644 $SHA_UU_ANC $SHA_UU_B $SHA_UU_A R0 conflict.txt\\n\" >>expected &&\n+\tprintf \"?? actual\\n\" >>expected &&\n+\tprintf \"?? expected\\n\" >>expected &&\n+\n+\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\tgit reset --hard &&\n+\ttest_cmp expected actual\n+'\n+\n+\n+##################################################################\n+## Test upstream fields in branch header\n+##################################################################\n+\n+test_expect_success 'upstream_fields_0' '\n+\tgit checkout master &&\n+\tgit clone . sub_repo &&\n+\t(\n+\t\t## Confirm local master tracks remote master.\n+\t\tcd sub_repo &&\n+\t\tHUF=`git rev-parse HEAD` &&\n+\t\tprintf \"## branch: $HUF master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual &&\n+\n+\t\t## Test ahead/behind.\n+\t\techo xyz >file_xyz &&\n+\t\tgit add file_xyz &&\n+\t\tgit commit -m xyz &&\n+\n+\t\tHUF=`git rev-parse HEAD` &&\n+\t\tprintf \"## branch: $HUF master origin/master +1 -0\\n\" >expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual &&\n+\n+\t\t## Test upstream-gone case. Fake this by pointing origin/master at\n+\t\t## a non-existing commit.\n+\t\tOLD=`git rev-parse origin/master` &&\n+\t\tNEW=$SHA_ZERO &&\n+\t\tmv .git/packed-refs .git/old-packed-refs &&\n+\t\tsed \"s/$OLD/$NEW/g\" <.git/old-packed-refs >.git/packed-refs &&\n+\n+\t\tHUF=`git rev-parse HEAD` &&\n+\t\tprintf \"## branch: $HUF master origin/master\\n\" >expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t) &&\n+\trm -rf sub_repo\n+'\n+\n+\n+##################################################################\n+## Test submodule status flags.\n+##################################################################\n+\n+test_expect_success 'submodule_flags_0' '\n+\tgit checkout master &&\n+\tgit clone . sub_repo &&\n+\tgit clone . super_repo &&\n+\t(\tcd super_repo &&\n+\t\tgit submodule add ../sub_repo sub1 &&\n+\n+\t\t## Confirm stage/add of clean submodule.\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"A. S 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_1' '\n+\t(\tcd super_repo &&\n+\t\t## Make some untracked dirt in the submodule.\n+\t\t(\tcd sub1 &&\n+\t\t\techo \"dirt\" >file_in_sub\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"AM SU 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_2' '\n+\t(\tcd super_repo &&\n+\t\t## Make some staged dirt in the submodule.\n+\t\t(\tcd sub1 &&\n+\t\t\tgit add file_in_sub\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"AM SM 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_3' '\n+\t(\tcd super_repo &&\n+\t\t## Make some staged and unstaged dirt (on the same file) in the submodule.\n+\t\t## This does not cause us to get SMU (because the submodule does not report\n+\t\t## a \"??\" line for the unstaged changes).\n+\t\t(\tcd sub1 &&\n+\t\t\techo \"more dirt\" >>file_in_sub\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"AM SM 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_4' '\n+\t(\tcd super_repo &&\n+\t\t## Make some staged and untracked dirt (on different files) in the submodule.\n+\t\t(\tcd sub1 &&\n+\t\t\tgit add file_in_sub &&\n+\t\t\techo \"dirt\" >>another_file_in_sub\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"AM SMU 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_5' '\n+\t(\tcd super_repo &&\n+\t\t## Make a new commit in the submodule.\n+\t\t(\tcd sub1 &&\n+\t\t\tgit add file_in_sub &&\n+\t\t\trm -f another_file_in_sub &&\n+\t\t\tgit commit -m \"new commit\"\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=$HSUP &&\n+\t\tprintf \"## branch: $HSUP master origin/master +0 -0\\n\" >expected &&\n+\t\tprintf \"A. N 000000 100644 100644 000000 $SHA_ZERO $HMOD $SHA_ZERO R0 .gitmodules\\n\" >>expected &&\n+\t\tprintf \"AM SC 000000 160000 160000 000000 $SHA_ZERO $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_6' '\n+\t(\tcd super_repo &&\n+\t\t## Commit the new submodule commit in the super.\n+\t\tgit add sub1 &&\n+\t\tgit commit -m \"super commit\" &&\n+\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tprintf \"## branch: $HSUP master origin/master +1 -0\\n\" >expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+test_expect_success 'submodule_flags_7' '\n+\t(\tcd super_repo &&\n+\t\t## Make some untracked dirt in the submodule.\n+\t\t(\tcd sub1 &&\n+\t\t\techo \"yet more dirt\" >>file_in_sub\n+\t\t) &&\n+\n+\t\tHMOD=`git hash-object -t blob -- .gitmodules` &&\n+\t\tHSUP=`git rev-parse HEAD` &&\n+\t\tHSUB=`(cd sub1 && git rev-parse HEAD)` &&\n+\t\tprintf \"## branch: $HSUP master origin/master +1 -0\\n\" >expected &&\n+\t\tprintf \".M SM 160000 160000 160000 000000 $HSUB $HSUB $SHA_ZERO R0 sub1\\n\" >>expected &&\n+\t\tprintf \"?? actual\\n\" >>expected &&\n+\t\tprintf \"?? expected\\n\" >>expected &&\n+\n+\t\tgit status --porcelain=2 --branch --ignored --untracked-files=all >actual &&\n+\t\ttest_cmp expected actual\n+\t)\n+'\n+\n+##################################################################\n+## The end.\n+##################################################################\n+\n+test_done\ndiff --git a/t/t7501-commit.sh b/t/t7501-commit.sh\nindex 1a74d75..753bae5 100755\n--- a/t/t7501-commit.sh\n+++ b/t/t7501-commit.sh\n@@ -77,6 +77,10 @@ test_expect_success '--porcelain=1 fails with nothing to commit' '\n \ttest_must_fail git commit -m initial --porcelain=1\n '\n \n+test_expect_success '--porcelain=2 fails with nothing to commit' '\n+\ttest_must_fail git commit -m initial --porcelain=2\n+'\n+\n test_expect_success '--long fails with nothing to commit' '\n \ttest_must_fail git commit -m initial --long\n '\n@@ -106,6 +110,11 @@ test_expect_failure '--porcelain=1 with stuff to commit returns ok' '\n \tgit commit -m next -a --porcelain=1\n '\n \n+test_expect_failure '--porcelain=2 with stuff to commit returns ok' '\n+\techo bongo bongo bongo >>file &&\n+\tgit commit -m next -a --porcelain=2\n+'\n+\n test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n \techo bongo bongo bongo >>file &&\n \tgit commit -m next -a --porcelain=bogus\n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291778","messageId":"1468966258-11191-6-git-send-email-jeffhost@microsoft.com","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"[PATCH v1 5/6] Add porcelain V2 documentation to status manpage","fromName":"Jeff Hostetler","fromEmail":"jeffhost@microsoft.com","sentAt":"2016-07-19T22:10:57Z","receivedAt":"2016-07-19T22:14:00Z","isPatch":true,"sender":{"key":"jeffhost@microsoft.com","avatar":"https://avatars.githubusercontent.com/u/7493832?v=4"},"body":"This commit updates the status manpage to include\ninformation about porcelain format V2.\n\nSigned-off-by: Jeff Hostetler \n---\n Documentation/git-status.txt | 62 +++++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 59 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\nindex de97729..01c42c0 100644\n--- a/Documentation/git-status.txt\n+++ b/Documentation/git-status.txt\n@@ -185,10 +185,10 @@ If -b is used the short-format status is preceded by a line\n \n ## branchname tracking info\n \n-Porcelain Format\n-~~~~~~~~~~~~~~~~\n+Porcelain Format Version 1\n+~~~~~~~~~~~~~~~~~~~~~~~~~~\n \n-The porcelain format is similar to the short format, but is guaranteed\n+Version 1 porcelain format is similar to the short format, but is guaranteed\n not to change in a backwards-incompatible way between Git versions or\n based on user configuration. This makes it ideal for parsing by scripts.\n The description of the short format above also describes the porcelain\n@@ -210,6 +210,62 @@ field from the first filename). Third, filenames containing special\n characters are not specially formatted; no quoting or\n backslash-escaping is performed.\n \n+Porcelain Format Version 2\n+~~~~~~~~~~~~~~~~~~~~~~~~~~\n+\n+\n+If `--branch` is given, a header line showing branch tracking information\n+is printed. This line begins with \"### branch: \". Fields are separated\n+by a single space.\n+\n+ Field Meaning\n+ --------------------------------------------------------\n+ | (initial) Current commit\n+ | (detached) Current branch\n+ Upstream branch, if set\n+ + Ahead count, if upstream present\n+ - Behind count, if upstream present\n+ --------------------------------------------------------\n+\n+A series of lines are then displayed for the tracked entries.\n+\n+ R [\\t]\n+\n+ Field Meaning\n+ --------------------------------------------------------\n+ The staged and unstaged values described earlier, with\n+ unchanged indicated by a \".\" rather than a space.\n+ The submodule state. \"N\" when the entry is not a submodule.\n+ \"S[C][M][U]\" when the entry is a submodule.\n+ \"C\" indicates the submodule commit has changed.\n+ \"M\" indicates the submodule has tracked changes.\n+ \"U\" indicates the submodule has untracked changes.\n+ The file modes for the entry.\n+ For unmerged entries, these are the stage 1, 2, and 3,\n+ and the worktree modes.\n+ For regular entries, these are the head, index, and\n+ worktree modes; the fourth is zero.\n+ The SHA1 values for the entry.\n+ For unmerged entries, these are the stage 1,2, and 3 values.\n+ For regular entries, these are the head and index values;\n+ the third entry is zero.\n+ R The rename percentage score.\n+ The current pathname. It is C-Quoted if necessary.\n+ The original path. This is only present for staged renames.\n+ It is C-Quoted if necessary.\n+ --------------------------------------------------------\n+\n+A series of lines are then displayed for untracked and ignored entries.\n+\n+ \n+\n+Where is \"??\" for untracked entries and \"!!\" for ignored entries.\n+\n+When the `-z` option is given, a NUL (zero) byte follows each pathname;\n+serving as both a separator and line termination. No pathname quoting\n+or backslash escaping is performed. All fields are output in the same\n+order.\n+\n CONFIGURATION\n -------------\n \n-- \n2.8.0.rc4.17.gac42084.dirty\n\n"},{"id":"291815","messageId":"alpine.DEB.2.20.1607201600050.14111@virtualbox","threadId":"42883","inReplyTo":"1468966258-11191-2-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-20T15:08:10Z","receivedAt":"2016-07-20T15:09:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jeff,\n\nOn Tue, 19 Jul 2016, Jeff Hostetler wrote:\n\n> Update the --porcelain argument to take an optional\n> version number. This will allow us to define new\n> porcelain formats in the future.\n> \n> This default to 1 and represents the existing porcelain\n> format.\n> \n> Signed-off-by: Jeff Hostetler \n\nVery nice introductory patch.\n\n> diff --git a/builtin/commit.c b/builtin/commit.c\n> index 1f6dbcd..892d7f7 100644\n> --- a/builtin/commit.c\n> +++ b/builtin/commit.c\n> @@ -142,14 +142,24 @@ static int show_ignored_in_status, have_option_m;\n> static const char *only_include_assumed;\n> static struct strbuf message = STRBUF_INIT;\n> \n> -static enum status_format {\n> -\tSTATUS_FORMAT_NONE = 0,\n> -\tSTATUS_FORMAT_LONG,\n> -\tSTATUS_FORMAT_SHORT,\n> -\tSTATUS_FORMAT_PORCELAIN,\n> +static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED;\n> \n> -\tSTATUS_FORMAT_UNSPECIFIED\n> -} status_format = STATUS_FORMAT_UNSPECIFIED;\n> +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tenum wt_status_format *value = (enum wt_status_format *)opt->value;\n> +\tif (unset) {\n> +\t\t*value = STATUS_FORMAT_UNSPECIFIED;\n\nIn Git's source code, we skip the curly braces for one-liners.\n\n> +\t} else if (arg) {\n> +\t\tint n = strtol(arg, NULL, 10);\n> +\t\tif (n == 1)\n> +\t\t\t*value = STATUS_FORMAT_PORCELAIN;\n> +\t\telse\n> +\t\t\tdie(\"unsupported porcelain version\");\n> +\t} else {\n> +\t\t*value = STATUS_FORMAT_PORCELAIN;\n\nThis could be folded into the previous conditional:\n\n\t}\n\telse {\n\t\tint n = arg ? strtol(arg, NULL, 10) : 1;\n\t\t...\n\nHaving said this, ...\n\n> @@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \t\t\t N_(\"show status concisely\"), STATUS_FORMAT_SHORT),\n> \t\tOPT_BOOL('b', \"branch\", &s.show_branch,\n> \t\t\t N_(\"show branch information\")),\n> -\t\tOPT_SET_INT(0, \"porcelain\", &status_format,\n> -\t\t\t N_(\"machine-readable output\"),\n> -\t\t\t STATUS_FORMAT_PORCELAIN),\n> +\t\t{ OPTION_CALLBACK, 0, \"porcelain\", &status_format,\n> +\t\t N_(\"version\"), N_(\"machine-readable output\"),\n> +\t\t PARSE_OPT_OPTARG, opt_parse_porcelain },\n\nHow about using a COUNTUP here instead? We could then set the status\nformat afterwards, like this:\n\n\tif (porcelain == 0)\n\t\tstatus_format = STATUS_FORMAT_UNSPECIFIED;\n\telse {\n\t\tstatus_format = STATUS_FORMAT_PORCELAIN;\n\t\tif (porcelain > 1)\n\t\t\twarning(\"No porcelain v%d; falling back to v1\",\n\t\t\t\tporcelain);\n\t}\n\nThe rest of the patch looks good to me!\n\nCiao,\nJohannes\n"},{"id":"291816","messageId":"alpine.DEB.2.20.1607201710240.14111@virtualbox","threadId":"42883","inReplyTo":"1468966258-11191-3-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-20T15:19:05Z","receivedAt":"2016-07-20T15:19:52Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jeff,\n\nOn Tue, 19 Jul 2016, Jeff Hostetler wrote:\n\n> Simple unit tests to validate the argument parsing.\n> \n> Signed-off-by: Jeff Hostetler \n\nThey are simple alright, but do we really need so many of them? I would\nlike to keep the ones in t7060, but I do not think that we necessarily\nhave to add the t7501 ones.\n\nI know I am a bit at odds here with Junio, who frequently prefers more\ntests. It's just that I have to run the complete test suite so often and\nit does take 30-45 minutes to run here (due to the fact that the test\nsuite exercises quite a lot of the POSIX emulation layer via shell\nscripting).\n\nSo do not take my suggestions as the sole basis for deciding how to go\nfrom here.\n\nCiao,\nJohannes\n"},{"id":"291817","messageId":"578F98BC.9060201@gmail.com","threadId":"42883","inReplyTo":"1468966258-11191-6-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 5/6] Add porcelain V2 documentation to status manpage","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-20T15:29:00Z","receivedAt":"2016-07-20T15:29:13Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n\n> +Porcelain Format Version 2\n> +~~~~~~~~~~~~~~~~~~~~~~~~~~\n> +\n> +\n> +If `--branch` is given, a header line showing branch tracking information\n> +is printed. This line begins with \"### branch: \". Fields are separated\n> +by a single space.\n> +\n> + Field Meaning\n> + --------------------------------------------------------\n> + | (initial) Current commit\n> + | (detached) Current branch\n\nI was wondering if all possible combinations are allowed. It turns out\nthat for technical implementation reasons it is not possible to have\n\"(initial) (detached)\".\n\nJust something I was wondering about, no need for any change...\n-- \nJakub Narębski\n"},{"id":"291818","messageId":"578F9923.3000403@gmail.com","threadId":"42883","inReplyTo":"1468966258-11191-7-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 6/6] Unit tests for V2 porcelain status","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-20T15:30:43Z","receivedAt":"2016-07-20T15:31:00Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n> +test_expect_success pre_initial_commit_0 '\n> +\tprintf \"## branch: (initial) master\\n\" >expected &&\n> +\tprintf \"?? actual\\n\" >>expected &&\n> +\tprintf \"?? dir1/\\n\" >>expected &&\n> +\tprintf \"?? expected\\n\" >>expected &&\n> +\tprintf \"?? file_x\\n\" >>expected &&\n> +\tprintf \"?? file_y\\n\" >>expected &&\n> +\tprintf \"?? file_z\\n\" >>expected &&\n\nWhy not use heredoc syntax (cat <<\\EOF), or prepare a file\nwith expected output in the testsuite?\n\n-- \nJakub Narębski\n"},{"id":"291819","messageId":"578F9AE8.4000407@jeffhostetler.com","threadId":"42883","inReplyTo":"alpine.DEB.2.20.1607201600050.14111@virtualbox","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T15:38:16Z","receivedAt":"2016-07-20T15:40:43Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 11:08 AM, Johannes Schindelin wrote:\n> On Tue, 19 Jul 2016, Jeff Hostetler wrote:\n>> diff --git a/builtin/commit.c b/builtin/commit.c\n>> +\t} else if (arg) {\n>> +\t\tint n = strtol(arg, NULL, 10);\n>> +\t\tif (n == 1)\n>> +\t\t\t*value = STATUS_FORMAT_PORCELAIN;\n>> +\t\telse\n>> +\t\t\tdie(\"unsupported porcelain version\");\n>> +\t} else {\n>> +\t\t*value = STATUS_FORMAT_PORCELAIN;\n>\n> This could be folded into the previous conditional:\n>\n> \t}\n> \telse {\n> \t\tint n = arg ? strtol(arg, NULL, 10) : 1;\n> \t\t...\n>\n\nI did it this way because I didn't want to make any assumptions\nhere on the numeric value of the enum values. Or rather, I didn't\nwant to add specific assignments to the enum type.\n\nThis also helps make it easier to see my later commit:\n else if (n == 2) *value = STATUS_FORMAT_PORCELAIN_V2;\n\nAlso, I didn't want to alter the order of assignments to the global\nstatus_format variable. That is, if I capture the value of in\na porcelain_version variable and assign that to status_format\nafterwards, there is opportunity for mistakes if they type:\n git status --short --porcelain=1 --long\nwhere the status_format variable is assigned 3 times.\n\n> Having said this, ...\n>\n>> @@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n>> \t\t\t N_(\"show status concisely\"), STATUS_FORMAT_SHORT),\n>> \t\tOPT_BOOL('b', \"branch\", &s.show_branch,\n>> \t\t\t N_(\"show branch information\")),\n>> -\t\tOPT_SET_INT(0, \"porcelain\", &status_format,\n>> -\t\t\t N_(\"machine-readable output\"),\n>> -\t\t\t STATUS_FORMAT_PORCELAIN),\n>> +\t\t{ OPTION_CALLBACK, 0, \"porcelain\", &status_format,\n>> +\t\t N_(\"version\"), N_(\"machine-readable output\"),\n>> +\t\t PARSE_OPT_OPTARG, opt_parse_porcelain },\n>\n> How about using a COUNTUP here instead? We could then set the status\n> format afterwards, like this:\n>\n> \tif (porcelain == 0)\n> \t\tstatus_format = STATUS_FORMAT_UNSPECIFIED;\n> \telse {\n> \t\tstatus_format = STATUS_FORMAT_PORCELAIN;\n> \t\tif (porcelain > 1)\n> \t\t\twarning(\"No porcelain v%d; falling back to v1\",\n> \t\t\t\tporcelain);\n> \t}\n>\n\nMaybe I misread the COUNTUP docs, but it looked like it would\nallow \"--porcelain --porcelain\", but not \"--porcelain=2\".\n\nJeff\n"},{"id":"291820","messageId":"578F9BC8.4040809@jeffhostetler.com","threadId":"42883","inReplyTo":"578F98BC.9060201@gmail.com","subject":"Re: [PATCH v1 5/6] Add porcelain V2 documentation to status manpage","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T15:42:00Z","receivedAt":"2016-07-20T15:44:33Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 11:29 AM, Jakub Narębski wrote:\n> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n>\n>> +Porcelain Format Version 2\n>> +~~~~~~~~~~~~~~~~~~~~~~~~~~\n>> +\n>> +\n>> +If `--branch` is given, a header line showing branch tracking information\n>> +is printed. This line begins with \"### branch: \". Fields are separated\n>> +by a single space.\n>> +\n>> + Field Meaning\n>> + --------------------------------------------------------\n>> + | (initial) Current commit\n>> + | (detached) Current branch\n>\n> I was wondering if all possible combinations are allowed. It turns out\n> that for technical implementation reasons it is not possible to have\n> \"(initial) (detached)\".\n>\n> Just something I was wondering about, no need for any change...\n>\n\nRight. I don't think that combination is possible. Not sure how\nto document that succinctly.\n\nJeff\n\n"},{"id":"291821","messageId":"578F9D2A.2030204@jeffhostetler.com","threadId":"42883","inReplyTo":"578F9923.3000403@gmail.com","subject":"Re: [PATCH v1 6/6] Unit tests for V2 porcelain status","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T15:47:54Z","receivedAt":"2016-07-20T15:50:25Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 11:30 AM, Jakub Narębski wrote:\n> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n>> +test_expect_success pre_initial_commit_0 '\n>> +\tprintf \"## branch: (initial) master\\n\" >expected &&\n>> +\tprintf \"?? actual\\n\" >>expected &&\n>> +\tprintf \"?? dir1/\\n\" >>expected &&\n>> +\tprintf \"?? expected\\n\" >>expected &&\n>> +\tprintf \"?? file_x\\n\" >>expected &&\n>> +\tprintf \"?? file_y\\n\" >>expected &&\n>> +\tprintf \"?? file_z\\n\" >>expected &&\n>\n> Why not use heredoc syntax (cat <<\\EOF), or prepare a file\n> with expected output in the testsuite?\n>\n\nThe tests involving renames needed to embed a tab character\nin the output and hiding a tab character in a heredoc seemed\nerror prone. So to be consistent I made them all printf-style.\n\nAlso, some of the tests include SHAs for the commit and for\nfile content, so having pre-computed expected output is awkward.\nGranted we could hard code the file SHAs, but not the commits.\n\nJeff\n"},{"id":"291822","messageId":"578F9DED.6010309@jeffhostetler.com","threadId":"42883","inReplyTo":"alpine.DEB.2.20.1607201710240.14111@virtualbox","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T15:51:09Z","receivedAt":"2016-07-20T15:53:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 11:19 AM, Johannes Schindelin wrote:\n> Hi Jeff,\n>\n> On Tue, 19 Jul 2016, Jeff Hostetler wrote:\n>\n>> Simple unit tests to validate the argument parsing.\n>>\n>> Signed-off-by: Jeff Hostetler \n>\n> They are simple alright, but do we really need so many of them? I would\n> like to keep the ones in t7060, but I do not think that we necessarily\n> have to add the t7501 ones.\n>\n> I know I am a bit at odds here with Junio, who frequently prefers more\n> tests. It's just that I have to run the complete test suite so often and\n> it does take 30-45 minutes to run here (due to the fact that the test\n> suite exercises quite a lot of the POSIX emulation layer via shell\n> scripting).\n>\n> So do not take my suggestions as the sole basis for deciding how to go\n> from here.\n\nI'm open to suggestion here. I mainly wanted to be able to\nprove that adding \"=1\" didn't affect the output and that an invalid\nparameter throws. We could eliminate several of the \"more trivial\"\nones if that would help.\n\nJeff\n\n"},{"id":"291823","messageId":"CANQwDwcjppbjfmgAf0pp_DHjF7pic153zkYHGeDKB70dxnw+9w@mail.gmail.com","threadId":"42883","inReplyTo":"578F9BC8.4040809@jeffhostetler.com","subject":"Re: [PATCH v1 5/6] Add porcelain V2 documentation to status manpage","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-20T15:55:09Z","receivedAt":"2016-07-20T15:56:07Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On 20 July 2016 at 17:42, Jeff Hostetler wrote:\n> On 07/20/2016 11:29 AM, Jakub Narębski wrote:\n>> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n>>\n>>> +Porcelain Format Version 2\n>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~\n>>> +\n>>> +\n>>> +If `--branch` is given, a header line showing branch tracking\n>>> information\n>>> +is printed. This line begins with \"### branch: \". Fields are separated\n>>> +by a single space.\n>>> +\n>>> + Field Meaning\n>>> + --------------------------------------------------------\n>>> + | (initial) Current commit\n>>> + | (detached) Current branch\n>>\n>> I was wondering if all possible combinations are allowed. It turns out\n>> that for technical implementation reasons it is not possible to have\n>> \"(initial) (detached)\".\n>>\n>> Just something I was wondering about, no need for any change...\n>\n> Right. I don't think that combination is possible. Not sure how\n> to document that succinctly.\n\nI don't think it is something we need to document, at least not here.\n\n-- \nJakub Narębski\n"},{"id":"291825","messageId":"20160720155837.GA24902@sigill.intra.peff.net","threadId":"42883","inReplyTo":"1468966258-11191-2-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T15:58:37Z","receivedAt":"2016-07-20T15:58:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 19, 2016 at 06:10:53PM -0400, Jeff Hostetler wrote:\n\n> +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)\n> +{\n> +\tenum wt_status_format *value = (enum wt_status_format *)opt->value;\n> +\tif (unset) {\n> +\t\t*value = STATUS_FORMAT_UNSPECIFIED;\n\nNice attention to detail here and below in handling \"unset\" and \"!arg\"\ncases. I think should be STATUS_FORMAT_NONE, though, which is what the\nold code used to do (since \"0\" is the usual special value for --no-*\noptions). It only matters if you do:\n\n git status --no-porcelain\n\nRight now that will switch to the long format, regardless of your\nconfig. With your path it defaults to any configured value. It's\nprobably a case that nobody hits ever, but in the absence of a good\nreason to do otherwise, I'd stick with the current behavior.\n\n> +\t} else if (arg) {\n> +\t\tint n = strtol(arg, NULL, 10);\n> +\t\tif (n == 1)\n> +\t\t\t*value = STATUS_FORMAT_PORCELAIN;\n> +\t\telse\n> +\t\t\tdie(\"unsupported porcelain version\");\n\nThis silently allows:\n\n git status --porcelain=\"1 for the money\"\n\nand later:\n\n git status --porcelain=\"2 for the show\"\n\nProbably not a big deal in practice, but since the list of formats is\nconstrained, we don't really care about parsing arbitrary numbers.\nSo:\n\n if (!strcmp(arg, \"1\"))\n\t*value = STATUS_FORMAT_PORCELAIN;\n\nis actually simpler, and more robust.\n\nI also wondered if:\n\n git status --porcelain=v1\n\nis more self-documenting about the meaning of \"1\". It's purely\naesthetics, but it somehow looks better to me. Matching that is also\nmuch easier with pure strcmps.\n\n> @@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n> \n> \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n> \ts.ignore_submodule_arg = ignore_submodule_arg;\n> +\ts.status_format = status_format;\n> +\n\nI wonder what happens if you pass a \"wt_status\" with a format of \"SHORT\"\nto the long-formatting code.\n\nI think it is ignored completely, as you are just now introducing the\ns.status_format field. But I wonder if there is room for further cleanup\nin pushing the big switch statements from run_status() and cmd_status()\ninto wt-status.c.\n\n-Peff\n"},{"id":"291826","messageId":"20160720160007.GB24902@sigill.intra.peff.net","threadId":"42883","inReplyTo":"1468966258-11191-3-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T16:00:08Z","receivedAt":"2016-07-20T16:00:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:\n\n> +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n> +\techo bongo bongo bongo >>file &&\n> +\tgit commit -m next -a --porcelain=bogus\n> +'\n\nHrm. That seems unexpected to me. Shouldn't it complain about\n--porcelain=bogus?\n\n-Peff\n"},{"id":"291828","messageId":"CANQwDwfWW4Fn7KBHrNv2nfvGHMrPQ4jGpz6wGAEGg3edn-ztfg@mail.gmail.com","threadId":"42883","inReplyTo":"578F9D2A.2030204@jeffhostetler.com","subject":"Re: [PATCH v1 6/6] Unit tests for V2 porcelain status","fromName":"Jakub Narębski","fromEmail":"jnareb@gmail.com","sentAt":"2016-07-20T16:01:44Z","receivedAt":"2016-07-20T16:02:30Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"On 20 July 2016 at 17:47, Jeff Hostetler wrote:\n> On 07/20/2016 11:30 AM, Jakub Narębski wrote:\n>> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n>>>\n>>> +test_expect_success pre_initial_commit_0 '\n>>> + printf \"## branch: (initial) master\\n\" >expected &&\n>>> + printf \"?? actual\\n\" >>expected &&\n>>> + printf \"?? dir1/\\n\" >>expected &&\n>>> + printf \"?? expected\\n\" >>expected &&\n>>> + printf \"?? file_x\\n\" >>expected &&\n>>> + printf \"?? file_y\\n\" >>expected &&\n>>> + printf \"?? file_z\\n\" >>expected &&\n>>\n>> Why not use heredoc syntax (cat <<\\EOF), or prepare a file\n>> with expected output in the testsuite?\n>\n> The tests involving renames needed to embed a tab character\n> in the output and hiding a tab character in a heredoc seemed\n> error prone. So to be consistent I made them all printf-style.\n\nAh, so that's the case for series of printf. I think in some other\ncases the Git testsuite simply uses HT variable for the TAB\ncharacter. I guess that \"\\t\" for TAB is available in POSIX and\nall shells that Git is run on?\n\nSee t3300-funny-names.sh, t3902-quoted.sh, t4213-log-tabexpand.sh\n\n> Also, some of the tests include SHAs for the commit and for\n> file content, so having pre-computed expected output is awkward.\n> Granted we could hard code the file SHAs, but not the commits.\n\nRight... but heredoc can include variable expansion (or rather\nit includes variable expansion by default, and you can prevent\nit by quoting end-of-heredoc marker).\n\n-- \nJakub Narębski\n"},{"id":"291829","messageId":"20160720160357.GA25076@sigill.intra.peff.net","threadId":"42883","inReplyTo":"20160720160007.GB24902@sigill.intra.peff.net","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T16:03:57Z","receivedAt":"2016-07-20T16:04:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 20, 2016 at 10:00:07AM -0600, Jeff King wrote:\n\n> On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:\n> \n> > +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n> > +\techo bongo bongo bongo >>file &&\n> > +\tgit commit -m next -a --porcelain=bogus\n> > +'\n> \n> Hrm. That seems unexpected to me. Shouldn't it complain about\n> --porcelain=bogus?\n\nPondering more, did you mean:\n\n test_expect_success '--porcelain=bogus complains about format' '\n\techo bongo bongo bongo >>file &&\n\ttest_must_fail git commit -m next -a --porcelain=bogus\n '\n\n?\n\nexpect_failure is for tests which we _want_ to succeed, but do not yet\n(so they get annotated in test results appropriately). expect_success is\nfor an outcome we expect to happen, but which may involve specific steps\nreturning failure.\n\nThe names are kind of confusing in that regard.\n\nI wonder if just \"test_expect\" would be a better name for\ntest_expect_success, and an argument or environment variable to trigger\n\"we know this is currently broken\" rather than having a separate\ntest_expect_failure function. That's clearly outside the scope of your\nseries, of course.\n\n-Peff\n"},{"id":"291830","messageId":"20160720160635.GC24902@sigill.intra.peff.net","threadId":"42883","inReplyTo":"1468966258-11191-5-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T16:06:36Z","receivedAt":"2016-07-20T16:07:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 19, 2016 at 06:10:56PM -0400, Jeff Hostetler wrote:\n\n> +\t} else {\n> +\t\t/*\n> +\t\t * TODO All of various print routines allow for s->branch to be null.\n> +\t\t * TODO When can this happen and what should we report here?\n> +\t\t */\n> +\t\tfprintf(s->fp, \" %s\", \"(unknown)\");\n> +\t}\n\nIIRC, it happens when HEAD points to a broken ref. So something like:\n\n git init\n echo broken >.git/refs/heads/master\n\nwould cause resolving HEAD to return NULL.\n\n-Peff\n"},{"id":"291832","messageId":"20160720161543.GD24902@sigill.intra.peff.net","threadId":"42883","inReplyTo":"1468966258-11191-1-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 0/6] Porcelain Status V2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T16:15:43Z","receivedAt":"2016-07-20T16:15:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 19, 2016 at 06:10:52PM -0400, Jeff Hostetler wrote:\n\n> This patch series adds porcelain V2 format to status.\n> This provides detailed information about file changes\n> and about the current branch.\n> \n> The new output is accessed via:\n> git status --porcelain=2 [--branch]\n> \n> An earlier draft of this work was submitted under the\n> \"Add very verbose porcelain output to status\" title.\n> This new version addresses the concerns about using\n> (abusing) \"-vv\" and simplifies the some of the formatting.\n\nI reviewed the first two, which look good except for a few minor\ncomments. I don't have time at the moment to dig carefully into the v2\nformat itself from the later patches (but from a cursory view they look\nOK). I'm flying all day today, so probably won't get to a more thorough\nreview for a day or two (but if there are other reviewers, please don't\nfeel you have to wait for my input).\n\nOne final bit of food for thought.\n\nJust yesterday somebody asked me about renewing the old idea of using a\nmore standardized format for machine-readable output, like --json.\nThat's obviously something that would exist alongside the existing\nformats for compatibility, and it doesn't fundamentally change anything\nabout adding a new format as your patches do (it just becomes yet\nanother format).\n\nHowever I wanted to mention it in case you are intrigued by the idea,\nand would be interested in skipping porcelain-v2 entirely in favor of\nmoving to something like json.\n\nA totally reasonable response is \"haha no. Please stop moving the\ngoalposts\". I just wanted to throw it out there as an option (and in\ncase you are interested, to let you think about it before any more work\ngoes into this direction).\n\n-Peff\n"},{"id":"291840","messageId":"578FB453.7080303@jeffhostetler.com","threadId":"42883","inReplyTo":"20160720155837.GA24902@sigill.intra.peff.net","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T17:26:43Z","receivedAt":"2016-07-20T17:29:05Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 11:58 AM, Jeff King wrote:\n> On Tue, Jul 19, 2016 at 06:10:53PM -0400, Jeff Hostetler wrote:\n>\n>> +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset)\n>> +{\n>> +\tenum wt_status_format *value = (enum wt_status_format *)opt->value;\n>> +\tif (unset) {\n>> +\t\t*value = STATUS_FORMAT_UNSPECIFIED;\n>\n> Nice attention to detail here and below in handling \"unset\" and \"!arg\"\n> cases. I think should be STATUS_FORMAT_NONE, though, which is what the\n> old code used to do (since \"0\" is the usual special value for --no-*\n> options). It only matters if you do:\n>\n> git status --no-porcelain\n>\n> Right now that will switch to the long format, regardless of your\n> config. With your path it defaults to any configured value. It's\n> probably a case that nobody hits ever, but in the absence of a good\n> reason to do otherwise, I'd stick with the current behavior.\n\nGood catch. I'll make it _NONE.\n\n>\n>> +\t} else if (arg) {\n>> +\t\tint n = strtol(arg, NULL, 10);\n>> +\t\tif (n == 1)\n>> +\t\t\t*value = STATUS_FORMAT_PORCELAIN;\n>> +\t\telse\n>> +\t\t\tdie(\"unsupported porcelain version\");\n>\n> This silently allows:\n>\n> git status --porcelain=\"1 for the money\"\n>\n> and later:\n>\n> git status --porcelain=\"2 for the show\"\n>\n> Probably not a big deal in practice, but since the list of formats is\n> constrained, we don't really care about parsing arbitrary numbers.\n> So:\n>\n> if (!strcmp(arg, \"1\"))\n> \t*value = STATUS_FORMAT_PORCELAIN;\n>\n> is actually simpler, and more robust.\n>\n> I also wondered if:\n>\n> git status --porcelain=v1\n>\n> is more self-documenting about the meaning of \"1\". It's purely\n> aesthetics, but it somehow looks better to me. Matching that is also\n> much easier with pure strcmps.\n\nI wondered about making it =v1 rather than just =1. It seemed\nmore aesthetically pleasing, even if it was an extra character\nto type. In a later email in this thread you mention a JSON\noption. If I switched this here to be \"=v1\" and \"=v2\", it would\nbe easy later to have a \"=j2\" or \"=v2j\" to do that.\n\n\n>\n>> @@ -1381,6 +1392,8 @@ int cmd_status(int argc, const char **argv, const char *prefix)\n>>\n>> \ts.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;\n>> \ts.ignore_submodule_arg = ignore_submodule_arg;\n>> +\ts.status_format = status_format;\n>> +\n>\n> I wonder what happens if you pass a \"wt_status\" with a format of \"SHORT\"\n> to the long-formatting code.\n>\n> I think it is ignored completely, as you are just now introducing the\n> s.status_format field. But I wonder if there is room for further cleanup\n> in pushing the big switch statements from run_status() and cmd_status()\n> into wt-status.c.\n\nI'll look into that.\n\nJeff\n"},{"id":"291841","messageId":"578FB4E6.1070105@jeffhostetler.com","threadId":"42883","inReplyTo":"20160720160007.GB24902@sigill.intra.peff.net","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T17:29:10Z","receivedAt":"2016-07-20T17:31:30Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 12:00 PM, Jeff King wrote:\n> On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:\n>\n>> +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n>> +\techo bongo bongo bongo >>file &&\n>> +\tgit commit -m next -a --porcelain=bogus\n>> +'\n>\n> Hrm. That seems unexpected to me. Shouldn't it complain about\n> --porcelain=bogus?\n\nLooks like a cut-n-paste operator error.\n\nJeff\n"},{"id":"291842","messageId":"578FB578.5070504@jeffhostetler.com","threadId":"42883","inReplyTo":"20160720160357.GA25076@sigill.intra.peff.net","subject":"Re: [PATCH v1 2/6] Status and checkout unit tests for --porcelain[=]","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T17:31:36Z","receivedAt":"2016-07-20T17:33:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 12:03 PM, Jeff King wrote:\n> On Wed, Jul 20, 2016 at 10:00:07AM -0600, Jeff King wrote:\n>\n>> On Tue, Jul 19, 2016 at 06:10:54PM -0400, Jeff Hostetler wrote:\n>>\n>>> +test_expect_failure '--porcelain=bogus with stuff to commit returns ok' '\n>>> +\techo bongo bongo bongo >>file &&\n>>> +\tgit commit -m next -a --porcelain=bogus\n>>> +'\n>>\n>> Hrm. That seems unexpected to me. Shouldn't it complain about\n>> --porcelain=bogus?\n>\n> Pondering more, did you mean:\n>\n> test_expect_success '--porcelain=bogus complains about format' '\n> \techo bongo bongo bongo >>file &&\n> \ttest_must_fail git commit -m next -a --porcelain=bogus\n> '\n>\n> ?\n\nYes. That is what I meant. And yes, some of the names are confusing.\nThanks.\n\nJeff\n"},{"id":"291847","messageId":"578FC0E8.4010102@jeffhostetler.com","threadId":"42883","inReplyTo":"20160720160635.GC24902@sigill.intra.peff.net","subject":"Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T18:20:24Z","receivedAt":"2016-07-20T18:22:56Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 12:06 PM, Jeff King wrote:\n> On Tue, Jul 19, 2016 at 06:10:56PM -0400, Jeff Hostetler wrote:\n>\n>> +\t} else {\n>> +\t\t/*\n>> +\t\t * TODO All of various print routines allow for s->branch to be null.\n>> +\t\t * TODO When can this happen and what should we report here?\n>> +\t\t */\n>> +\t\tfprintf(s->fp, \" %s\", \"(unknown)\");\n>> +\t}\n>\n> IIRC, it happens when HEAD points to a broken ref. So something like:\n>\n> git init\n> echo broken >.git/refs/heads/master\n>\n> would cause resolving HEAD to return NULL.\n\nThat worked and I see \"(unknown)\".\n\nThis is a bit of a nit, but is there a value we'd like\nto see there, such as \"(unknown)\" or \"(broken)\" or \"(missing)\"\nin that case? (And make it clear that this is a different\ncase from \"(detached)\".)\n\nI'm thinking it would be nicer to always have a field\nthere for parsing.\n\nJeff\n\n\n\n\n"},{"id":"291851","messageId":"578FD0B1.9030709@jeffhostetler.com","threadId":"42883","inReplyTo":"20160720161543.GD24902@sigill.intra.peff.net","subject":"Re: [PATCH v1 0/6] Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-20T19:27:45Z","receivedAt":"2016-07-20T19:30:07Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 12:15 PM, Jeff King wrote:\n> One final bit of food for thought.\n>\n> Just yesterday somebody asked me about renewing the old idea of using a\n> more standardized format for machine-readable output, like --json.\n> That's obviously something that would exist alongside the existing\n> formats for compatibility, and it doesn't fundamentally change anything\n> about adding a new format as your patches do (it just becomes yet\n> another format).\n>\n> However I wanted to mention it in case you are intrigued by the idea,\n> and would be interested in skipping porcelain-v2 entirely in favor of\n> moving to something like json.\n>\n> A totally reasonable response is \"haha no. Please stop moving the\n> goalposts\". I just wanted to throw it out there as an option (and in\n> case you are interested, to let you think about it before any more work\n> goes into this direction).\n\nhaha no.... :-)\n\nShort term, I'd rather nail down what I have now (both content-wise\nand format-wise) and see how we like it. And have a follow-up task\nto look at the --state header we spoke of earlier. And save the JSON\nversion as an independent task for later.\n\nI understand the motivation for a JSON option (and have thought\nabout it before) but I think it ought to be kept separate.\nAt a higher-level, it seems like a JSON option would be an\nopportunity to start a project-wide conversation about formats,\nconsistency, plumbing, and etc. A top-down conversation if you\nwill about which commands will/won't get enhanced, legacy cruft\nthat would not need to be converted, JSON style and naming and\nconsistency issues, current best practices in the node/whatever\ncommunity, and etc. I could be wrong, but this feels like a\ntop-down feature conversation in a wider audience.\n\nJeff\n\n\n"},{"id":"291856","messageId":"xmqqshv4ca1z.fsf@gitster.mtv.corp.google.com","threadId":"42883","inReplyTo":"1468966258-11191-2-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-20T20:46:16Z","receivedAt":"2016-07-20T20:46:27Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\n> index b0a294d..0791573 100644\n> --- a/Documentation/git-commit.txt\n> +++ b/Documentation/git-commit.txt\n> @@ -104,7 +104,7 @@ OPTIONS\n> --branch::\n> \tShow the branch and tracking info even in short-format.\n> \n> ---porcelain::\n> +--porcelain[=]::\n> \tWhen doing a dry-run, give the output in a porcelain-ready\n> \tformat. See linkgit:git-status[1] for details. Implies\n> \t`--dry-run`.\n\nI suspect that it would be easier to implement to allow \"commit --porcelain\",\nbut I am not sure if it truly makes sense. Not a new problem, though.\n\n> diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt\n> index e1e8f57..de97729 100644\n> --- a/Documentation/git-status.txt\n> +++ b/Documentation/git-status.txt\n> @@ -32,11 +32,14 @@ OPTIONS\n> --branch::\n> \tShow the branch and tracking info even in short-format.\n> \n> ---porcelain::\n> +--porcelain[=]::\n> \tGive the output in an easy-to-parse format for scripts.\n> \tThis is similar to the short output, but will remain stable\n> \tacross Git versions and regardless of user configuration. See\n> \tbelow for details.\n> ++\n> +The version parameter is used to specify the format version.\n> +This is optional and defaults to the original version '1' format.\n\nI agree with Peff that \"v1\" would be easier to read, and also his\ncomment on parsing using strcmp() to require exact matches and\nresetting to NONE, not UNSPECIFIED.\n"},{"id":"291857","messageId":"xmqqoa5sc9ud.fsf@gitster.mtv.corp.google.com","threadId":"42883","inReplyTo":"1468966258-11191-4-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-20T20:50:50Z","receivedAt":"2016-07-20T20:50:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\nJust to avoid later headaches... please look at your commit titles\nand imagine how they will look when listed among 400+ other changes\nwhen they are included in a future release in \"git shortlog\" output.\n\n> Subject: Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2\n\nSubject: status: per-file output for --porcelain=v2\n\nor something like that, perhaps?\n\n> This commit sets up version 2 porcelain status and\n> defines the format of detail lines. This includes\n> the usual XY and pathname fields. It adds the various\n> file modes and SHAs and the rename score. For regular\n> entries these values reflect the head, index and\n> worktree. For unmerged entries these values reflect\n> the stage 1, 2, and 3 values.\n\nAlso, we usually do not say \"This commit does this and that\".\n\nSee Documentation/SubmittingPatches for more details regarding the\nabove two points (and more).\n\n"},{"id":"291860","messageId":"20160720205411.GB578@sigill.intra.peff.net","threadId":"42883","inReplyTo":"578FC0E8.4010102@jeffhostetler.com","subject":"Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T20:54:12Z","receivedAt":"2016-07-20T20:54:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 20, 2016 at 02:20:24PM -0400, Jeff Hostetler wrote:\n\n> > IIRC, it happens when HEAD points to a broken ref. So something like:\n> > \n> > git init\n> > echo broken >.git/refs/heads/master\n> > \n> > would cause resolving HEAD to return NULL.\n> \n> That worked and I see \"(unknown)\".\n> \n> This is a bit of a nit, but is there a value we'd like\n> to see there, such as \"(unknown)\" or \"(broken)\" or \"(missing)\"\n> in that case? (And make it clear that this is a different\n> case from \"(detached)\".)\n> \n> I'm thinking it would be nicer to always have a field\n> there for parsing.\n\nMy gut feeling is to err on the side of being vague, like \"unknown\".\nThis is something that _shouldn't_ ever happen, and if it does, it could\nbe a broken on-disk ref, a transient syscall error, or some other\nweirdness. I don't think we need to get too specific in this context\n(we'll likely have said something else useful on stderr already, I\nthink).\n\n-Peff\n"},{"id":"291861","messageId":"20160720205753.GC578@sigill.intra.peff.net","threadId":"42883","inReplyTo":"578FD0B1.9030709@jeffhostetler.com","subject":"Re: [PATCH v1 0/6] Porcelain Status V2","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-07-20T20:57:53Z","receivedAt":"2016-07-20T20:58:01Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jul 20, 2016 at 03:27:45PM -0400, Jeff Hostetler wrote:\n\n> > A totally reasonable response is \"haha no. Please stop moving the\n> > goalposts\". I just wanted to throw it out there as an option (and in\n> > case you are interested, to let you think about it before any more work\n> > goes into this direction).\n> \n> haha no.... :-)\n> \n> Short term, I'd rather nail down what I have now (both content-wise\n> and format-wise) and see how we like it. And have a follow-up task\n> to look at the --state header we spoke of earlier. And save the JSON\n> version as an independent task for later.\n> \n> I understand the motivation for a JSON option (and have thought\n> about it before) but I think it ought to be kept separate.\n> At a higher-level, it seems like a JSON option would be an\n> opportunity to start a project-wide conversation about formats,\n> consistency, plumbing, and etc. A top-down conversation if you\n> will about which commands will/won't get enhanced, legacy cruft\n> that would not need to be converted, JSON style and naming and\n> consistency issues, current best practices in the node/whatever\n> community, and etc. I could be wrong, but this feels like a\n> top-down feature conversation in a wider audience.\n\nI agree with everything you've said here.\n\nIf we add JSON, we'd want to do it everywhere: lists of commits, lists\nof refs, status output, etc. I mentioned that somebody had asked me\nabout it recently; they are working on a git client and finding that\nlibgit2 is not serving their needs well, so they'd like to shell out to\ngit more, and wanted to have a standard way to get the data back in.\n\n-Peff\n"},{"id":"291873","messageId":"xmqqh9bkc7xx.fsf@gitster.mtv.corp.google.com","threadId":"42883","inReplyTo":"1468966258-11191-4-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-20T21:31:54Z","receivedAt":"2016-07-20T21:32:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> diff --git a/wt-status.c b/wt-status.c\n> index c19b52c..2d4829e 100644\n> --- a/wt-status.c\n> +++ b/wt-status.c\n> @@ -406,6 +406,89 @@ static void wt_status_print_change_data(struct wt_status *s,\n> \tstrbuf_release(&twobuf);\n> }\n> \n> +\n> +/* Copy info for both sides of a head-vs-index change\n> + * into the Porcelain V2 data.\n> + */\n\n\t/* \n\t * Please don't use unbalanced/asymmetric comment.\n * Our multi-line comment blocks look more like\n * this.\n */\n\n> +static void porcelain_v2_updated_entry(\n> +\tstruct wt_status_change_data *d,\n> +\tstruct diff_filepair *p)\n> +{\n> +\tswitch (p->status) {\n> +\tcase DIFF_STATUS_ADDED:\n> +\t\td->porcelain_v2.mode_index = p->two->mode;\n> +\t\thashcpy(d->porcelain_v2.sha1_index, p->two->sha1);\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_DELETED:\n> +\t\td->porcelain_v2.mode_head = p->one->mode;\n> +\t\thashcpy(d->porcelain_v2.sha1_head, p->one->sha1);\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_RENAMED:\n> +\t\td->porcelain_v2.rename_score = p->score * 100 / MAX_SCORE;\n> +\tcase DIFF_STATUS_COPIED:\n> +\tcase DIFF_STATUS_MODIFIED:\n> +\tcase DIFF_STATUS_TYPE_CHANGED:\n> +\tcase DIFF_STATUS_UNMERGED:\n> +\t\td->porcelain_v2.mode_head = p->one->mode;\n> +\t\td->porcelain_v2.mode_index = p->two->mode;\n> +\t\thashcpy(d->porcelain_v2.sha1_head, p->one->sha1);\n> +\t\thashcpy(d->porcelain_v2.sha1_index, p->two->sha1);\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_UNKNOWN:\n> +\t\t/* This should never happen. */\n> +\t\tbreak;\n> +\t}\n\nYou may want to have\n\n\tdie(\"BUG: status unknown???\");\n\nif you know \"This should never happen.\".\n\nThe code seems to assume that d->porcelain_v2.* fields are\ninitialized earlier in the callchain to reasonable values\n(e.g. STATUS_ADDED case does not clear .mode_head to \"missing\"); I\nam not sure if that is easier to read or fill in all the values\nexplicitly in this function.\n\n> +}\n> +\n> +/* Copy info for both sides of an index-vs-worktree change\n> + * into the very verbose porcelain data.\n> + */\n> +static void porcelain_v2_changed_entry(\n> +\tstruct wt_status_change_data *d,\n> +\tconst struct diff_filepair *p)\n> +{\n> +\tswitch (p->status) {\n> +\tcase DIFF_STATUS_ADDED:\n> +\t\td->porcelain_v2.mode_worktree = p->two->mode;\n> +\t\t/* don't bother with worktree sha, since it is almost always zero. */\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_DELETED:\n> +\t\td->porcelain_v2.mode_index = p->one->mode;\n> +\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_COPIED:\n> +\tcase DIFF_STATUS_MODIFIED:\n> +\tcase DIFF_STATUS_RENAMED:\n\nCan RENAMED or COPIED happen here?\n\n> +\tcase DIFF_STATUS_TYPE_CHANGED:\n> +\t\td->porcelain_v2.mode_index = p->one->mode;\n> +\t\td->porcelain_v2.mode_worktree = p->two->mode;\n> +\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n> +\t\t/* don't bother with worktree sha, since it is almost always zero. */\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_UNKNOWN:\n> +\t\t/* This should never happen. */\n> +\t\tbreak;\n> +\n> +\tcase DIFF_STATUS_UNMERGED:\n> +\t\t/* This should never happen. */\n> +\t\tbreak;\n> +\t}\n> +}\n\nOther than that, the same comments as the ones for _updated_ apply\nto this _changed_ function.\n\n> +static void porcelain_v2_added_initial_entry(\n> +\tstruct wt_status_change_data *d,\n> +\tconst struct cache_entry *ce)\n> +{\n> +\td->porcelain_v2.mode_index = ce->ce_mode;\n> +\thashcpy(d->porcelain_v2.sha1_index, ce->sha1);\n> +}\n> +\n> static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\t\t\t\t struct diff_options *options,\n> \t\t\t\t\t void *data)\n> @@ -433,6 +516,9 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n> \t\td->dirty_submodule = p->two->dirty_submodule;\n> \t\tif (S_ISGITLINK(p->two->mode))\n> \t\t\td->new_submodule_commits = !!hashcmp(p->one->sha1, p->two->sha1);\n> +\n> +\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\t\tporcelain_v2_changed_entry(d, p);\n> \t}\n> }\n> \n> @@ -486,6 +572,9 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n> \t\t\td->stagemask = unmerged_mask(p->two->path);\n> \t\t\tbreak;\n> \t\t}\n> +\n> +\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\t\tporcelain_v2_updated_entry(d, p);\n> \t}\n> }\n> @@ -565,8 +654,12 @@ static void wt_status_collect_changes_initial(struct wt_status *s)\n> \t\t\td->index_status = DIFF_STATUS_UNMERGED;\n> \t\t\td->stagemask |= (1 << (ce_stage(ce) - 1));\n> \t\t}\n> -\t\telse\n> +\t\telse {\n> \t\t\td->index_status = DIFF_STATUS_ADDED;\n> +\n> +\t\t\tif (s->status_format == STATUS_FORMAT_PORCELAIN_V2)\n> +\t\t\t\tporcelain_v2_added_initial_entry(d, ce);\n> +\t\t}\n> \t}\n> }\n\nThe code would be simpler and we'll catch bugs more uniformly if\n\"collect\" phase is made oblivious to s->status_format, I would\nsuspect, by making unconditional call to porcelain_v2_*_entry()\nfunctions from all of these \"collect\" functions.\n\n> +/* Convert various submodule status values into a\n> + * string of characters in the buffer provided.\n> + */\n> +static void wt_porcelain_v2_submodule_state(\n> +\tstruct wt_status_change_data *d,\n> +\tchar sub[5])\n> +{\n> +\tint k = 0;\n> +\n> +\tif (S_ISGITLINK(d->porcelain_v2.mode_head) ||\n> +\t\tS_ISGITLINK(d->porcelain_v2.mode_index) ||\n> +\t\tS_ISGITLINK(d->porcelain_v2.mode_worktree)) {\n> +\t\t/* We have a submodule */\n> +\t\tsub[k++] = 'S';\n> +\n> +\t\t/* Sub-flags for each type of dirt */\n> +\t\tif (d->new_submodule_commits)\n> +\t\t\tsub[k++] = 'C';\n> +\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n> +\t\t\tsub[k++] = 'M';\n> +\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n> +\t\t\tsub[k++] = 'U';\n> +\t} else {\n> +\t\t/* Not a submodule */\n> +\t\tsub[k++] = 'N';\n> +\t}\n> +\n> +\tsub[k] = 0;\n> +}\n\nThe above seems to do \"up to 5\"; if these come early in the output,\nit may instead be easier to read if \"fixed 5 columns, many of them\nmay be SP, and some may be C or M or U\", which would align on fixed\ncolumn terminal people use for coding.\n\n> +/* Various fix-up steps before we start printing an item.\n> + */\n> +static void wt_porcelain_v2_fix_up_status(\n> +\tstruct string_list_item *it,\n> +\tstruct wt_status *s)\n> +{\n> +\tstruct wt_status_change_data *d = it->util;\n> +\n> +\tif (!d->index_status) {\n> +\t\tif (d->worktree_status == DIFF_STATUS_MODIFIED ||\n> +\t\t\td->worktree_status == DIFF_STATUS_DELETED) {\n> +\t\t\t/* X=' ' Y=[MD]\n> +\t\t\t * The item did not change in head-vs-index scan so the head\n> +\t\t\t * column was never set. (The index column was set during the\n> +\t\t\t * index-vs-worktree scan.)\n> +\t\t\t * Force set the head column to make the output complete.\n> +\t\t\t */\n> +\t\t\td->porcelain_v2.mode_head = d->porcelain_v2.mode_index;\n> +\t\t\thashcpy(d->porcelain_v2.sha1_head, d->porcelain_v2.sha1_index);\n> +\t\t}\n\nCan there be \"else\" clause for this inner \"if\"? When\nd->index_status is not set, but worktree_status is not one of these\ntwo, what does it indicate? The same for the next one.\n\n> +\t}\n> +\n> +\tif (!d->worktree_status) {\n> +\t\tif (d->index_status == DIFF_STATUS_MODIFIED ||\n> +\t\t\td->index_status == DIFF_STATUS_ADDED ||\n> +\t\t\td->index_status == DIFF_STATUS_RENAMED ||\n> +\t\t\td->index_status == DIFF_STATUS_COPIED) {\n> +\t\t\t/* X=[MARC] Y=' '\n> +\t\t\t * The item did not changed in the index-vs-worktree scan so\n\ns/changed/change/\n\n> +\t\t\t * the worktree column was never set.\n> +\t\t\t * Force set the worktree mode to make the output complete.\n> +\t\t\t */\n> +\t\t\td->porcelain_v2.mode_worktree = d->porcelain_v2.mode_index;\n> +\t\t}\n> +\t}\n> +}\n> +\n> +/*\n> + * Define a single format for tracked entries. This includes:\n> + * normal changes, rename changes, and unmerged changes.\n> + *\n> + * The meanings of modes_[abcd] and sha_[abc] depends on the\n> + * change type, but are always present.\n> + *\n> + * Path(s) are C-Quoted if necessary. Current path is ALWAYS\n> + * first. The rename source path is only present when necessary.\n> + * A single TAB separates them (because paths can contain spaces\n> + * and C-Quoting converts actual tabs in pathnames to a C escape\n> + * sequence).\n> + */\n> +static void wt_porcelain_v2_print_tracked_entry(\n> +\tFILE *fp,\n> +\tchar x_staged,\n> +\tchar y_unstaged,\n> +\tconst char *submodule,\n> +\tint mode_a,\n> +\tint mode_b,\n> +\tint mode_c,\n> +\tint mode_d,\n> +\tconst unsigned char sha_a[GIT_SHA1_RAWSZ],\n> +\tconst unsigned char sha_b[GIT_SHA1_RAWSZ],\n> +\tconst unsigned char sha_c[GIT_SHA1_RAWSZ],\n\nx, y, a, b, c, d are all mystery. When we see a/b/c, they typically\nmean stage#1, stage#2 and stage#3, but that does not seem to be it.\nWhen we see a/b, they typically mean old and new, but that does not\nseem to be the case here, either.\n\nIf you are saying in the above comment, which curiously is formatted\ncorrectly unlike many other multi-line comments we see in this patch\n;-), that the meaning of these are not fixed but the function is to\nalways show 2 things, then four things in the fixed order without\nknowing what they mean, it would be easier to understand if you made\nthem into an array, i.e.e.g. instead of having x_staged and\ny_unstaged, have \"char prefix[2]\" or something that tells the reader\nthat whatever meaning these have, these come early in the output.\n\n> +\tint rename_score,\n> +\tconst char *path_current,\n> +\tconst char *path_rename_src,\n> +\tint null_termination)\n> +{\n> +\tchar sep_char = null_termination ? '\\0' : '\\t';\n> +\tchar eol_char = null_termination ? '\\0' : '\\n';\n\nMental note (i.e. shouldn't be done as part of this patch, but is\nsomething we should fix, probably as a preparatory clean-up patch\nbefore the series): these should be spelled nul_termination, as the\nname of the '\\0' character is NUL, not NULL.\n\n> +\tif (path_rename_src)\n> +\t\tfprintf(fp, \"%c%c %s %06o %06o %06o %06o %s %s %s R%d %s%c%s%c\",\n> +\t\t\t\tx_staged, y_unstaged, submodule,\n> +\t\t\t\tmode_a, mode_b, mode_c, mode_d,\n> +\t\t\t\tsha1_to_hex(sha_a), sha1_to_hex(sha_b), sha1_to_hex(sha_c),\n> +\t\t\t\trename_score,\n> +\t\t\t\tpath_current, sep_char, path_rename_src,\n> +\t\t\t\teol_char);\n> +\telse\n> +\t\tfprintf(fp, \"%c%c %s %06o %06o %06o %06o %s %s %s R%d %s%c\",\n> +\t\t\t\tx_staged, y_unstaged, submodule,\n> +\t\t\t\tmode_a, mode_b, mode_c, mode_d,\n> +\t\t\t\tsha1_to_hex(sha_a), sha1_to_hex(sha_b), sha1_to_hex(sha_c),\n> +\t\t\t\trename_score,\n> +\t\t\t\tpath_current,\n> +\t\t\t\teol_char);\n> +}\n> +\n> +/*\n> + * Print porcelain V2 info for normal tracked entries.\n> + */\n> +static void wt_porcelain_v2_print_normal_entry(\n> +\tstruct string_list_item *it,\n> +\tstruct wt_status *s)\n> +{\n> +\tstatic const unsigned char sha_zero[GIT_SHA1_RAWSZ] = {0};\n> +\tstruct wt_status_change_data *d = it->util;\n> +\tstruct strbuf buf_current = STRBUF_INIT;\n> +\tstruct strbuf buf_rename_src = STRBUF_INIT;\n> +\tconst char *path_current = NULL;\n> +\tconst char *path_rename_src = NULL;\n> +\tchar x_staged, y_unstaged;\n> +\tchar submodule[5];\n> +\n> +\twt_porcelain_v2_fix_up_status(it, s);\n> +\tx_staged = d->index_status ? d->index_status : '.';\n> +\ty_unstaged = d->worktree_status ? d->worktree_status : '.';\n\nI see a curious and misleading use of the word \"staged\" around\nhere. If these two are index-status and worktree-status (with\nstand-in letter '.' when they are not available), why not call them\nindex-status and worktree-status instead?\n\n> +/*\n> + * Print very verbose porcelain status info for unmerged entries.\n> + */\n> +static void wt_porcelain_v2_print_unmerged_entry(\n> +\tstruct string_list_item *it,\n> +\tstruct wt_status *s)\n> +{\n> +\tstruct wt_status_change_data *d = it->util;\n> +\tconst struct cache_entry *ce;\n> +\tstruct strbuf buf_current = STRBUF_INIT;\n> +\tconst char *path_current = NULL;\n> +\tint pos, stage;\n> +\tstruct {\n> +\t\tint mode;\n> +\t\tunsigned char sha1[GIT_SHA1_RAWSZ];\n> +\t} stages[3];\n> +\tchar x_us = 'U', y_them = 'U';\n\nI really do not want to see these X_ and Y_ prefixes; they do not\nconvey any meaning; if X always corresponds to ours while Y always\ncorresponds to theirs, then they are even redundant.\n\n> +\tchar submodule[5];\n> +\n> +\tswitch (d->stagemask) {\n> +\tcase 1: x_us = 'D'; y_them = 'D'; break; /* both deleted */\n> +\tcase 2: x_us = 'A'; y_them = 'U'; break; /* added by us */\n> +\tcase 3: x_us = 'U'; y_them = 'D'; break; /* deleted by them */\n> +\tcase 4: x_us = 'U'; y_them = 'A'; break; /* added by them */\n> +\tcase 5: x_us = 'D'; y_them = 'U'; break; /* deleted by us */\n> +\tcase 6: x_us = 'A'; y_them = 'A'; break; /* both added */\n> +\tcase 7: x_us = 'U'; y_them = 'U'; break; /* both modified */\n> +\t}\n> +\n> +\twt_porcelain_v2_submodule_state(d, submodule);\n> +\n> +\t/*\n> +\t * Disregard the V2 {mode,sha} values for head and index\n> +\t * that we computed from the diffs and lookup the actual\n> +\t * stage data.\n> +\t */\n> +\tmemset(stages, 0, sizeof(stages));\n> +\tpos = cache_name_pos(it->string, strlen(it->string));\n> +\tassert(pos < 0);\n> +\tpos = -pos-1;\n> +\twhile (pos < active_nr) {\n> +\t\tce = active_cache[pos++];\n> +\t\tstage = ce_stage(ce);\n> +\t\tif (strcmp(ce->name, it->string) || !stage)\n> +\t\t\tbreak;\n> +\t\tstages[stage - 1].mode = ce->ce_mode;\n> +\t\thashcpy(stages[stage - 1].sha1, ce->sha1);\n> +\t}\n\nYou did assert(pos < 0) to make sure that the path the caller told\nyou is unmerged indeed is unmerged, which is a very good check. In\nthe same spirit, you would want to make sure that you got at least\none result from the above loop, to diagnose a bug where the path did\nnot even exist at all in the index.\n\nAlso, it is possible to have more than one stage#1 entries in the\nindex (stage#2 can only be one; I think stage#3 can theoretically\nhave more than one, but in practice, even merge-octopus does not do\nthat). The above loop gives us the \"last one wins\" semantics to\nmake sure what you grab from the index always fit in stages[] array,\nwhich is not wrong per-se, but that design decision may want to be\ndocumented in a comment nearby for people to work on enhancing it in\nthe future.\n\n"},{"id":"291881","messageId":"xmqqd1m8c73h.fsf@gitster.mtv.corp.google.com","threadId":"42883","inReplyTo":"1468966258-11191-6-git-send-email-jeffhost@microsoft.com","subject":"Re: [PATCH v1 5/6] Add porcelain V2 documentation to status manpage","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-20T21:50:10Z","receivedAt":"2016-07-20T21:50:19Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n> +A series of lines are then displayed for the tracked entries.\n> +\n> + R [\\t]\n> +\n> + Field Meaning\n> + --------------------------------------------------------\n> + The staged and unstaged values described earlier, with\n> + unchanged indicated by a \".\" rather than a space.\n\nAhh, this is where these mysterious xy came from. You just needed\ntwo random consecutive letters, and they could have been ab, ij, or\njk. I don't like any of them ;-)\n\nAlso I have trouble with the \"staged and unstaged\" here, especially\nthe latter, as the word implies the user did \"git rm --cached\" on\nthe path earlier, which is not the case. You are saying what is in\nthe index and what is in the working tree.\n\nPerhaps using iw instead of xy to make them in sync with the way\n\"git diff --mnemonic-prefix\" denotes the contents in the index and\nin the working tree? Together with s/staged/in the index/ and\ns/unstaged/in the working tree/, the result would become more\nconsistent with the rest of the system, I suspect.\n\n> + The file modes for the entry.\n> + For unmerged entries, these are the stage 1, 2, and 3,\n> + and the worktree modes.\n> + For regular entries, these are the head, index, and\n> + worktree modes; the fourth is zero.\n> + The SHA1 values for the entry.\n> + For unmerged entries, these are the stage 1,2, and 3 values.\n> + For regular entries, these are the head and index values;\n> + the third entry is zero.\n\nTo future-proof, we should use \"object name\" for what you call \"SHA1\nvalue\" here, I would think.\n\n> + R The rename percentage score.\n\ns/percentage score/score/. Do you differentiate between renames and\ncopies?\n\n> + The current pathname. It is C-Quoted if necessary.\n> + The original path. This is only present for staged renames.\n> + It is C-Quoted if necessary.\n\nSeeing \"if necessary\" makes me wonder if we want to define when it\nis \"necessary\".\n"},{"id":"291915","messageId":"alpine.DEB.2.20.1607211616320.14111@virtualbox","threadId":"42883","inReplyTo":"xmqqoa5sc9ud.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T14:19:13Z","receivedAt":"2016-07-21T14:20:17Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Jul 2016, Junio C Hamano wrote:\n\n> Jeff Hostetler writes:\n> \n> Just to avoid later headaches... please look at your commit titles\n> and imagine how they will look when listed among 400+ other changes\n> when they are included in a future release in \"git shortlog\" output.\n> \n> > Subject: Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2\n> \n> Subject: status: per-file output for --porcelain=v2\n> \n> or something like that, perhaps?\n> \n> > This commit sets up version 2 porcelain status and\n> > defines the format of detail lines. This includes\n> > the usual XY and pathname fields. It adds the various\n> > file modes and SHAs and the rename score. For regular\n> > entries these values reflect the head, index and\n> > worktree. For unmerged entries these values reflect\n> > the stage 1, 2, and 3 values.\n> \n> Also, we usually do not say \"This commit does this and that\".\n> \n> See Documentation/SubmittingPatches for more details regarding the\n> above two points (and more).\n\nMaybe something like this:\n\n-- snipsnap --\nstatus: per-file output for --porcelain=2\n\nThe output of `git status --porcelain` leaves out many details that,\nsay, an IDE would need to know about the current status.\n\nLet's introduce version porcelain status v2 that adds the various file\nmodes and SHAs and the rename score. For regular entries these values\nreflect the head, index and worktree. For unmerged entries these values\nreflect the stage 1, 2, and 3 values.\n"},{"id":"291916","messageId":"alpine.DEB.2.20.1607211624190.14111@virtualbox","threadId":"42883","inReplyTo":"578F9AE8.4000407@jeffhostetler.com","subject":"Re: [PATCH v1 1/6] Allow --porcelain[=] in status and commit commands","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T14:28:26Z","receivedAt":"2016-07-21T14:29:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Jeff,\n\nOn Wed, 20 Jul 2016, Jeff Hostetler wrote:\n\n> On 07/20/2016 11:08 AM, Johannes Schindelin wrote:\n> > On Tue, 19 Jul 2016, Jeff Hostetler wrote:\n> > > @@ -1336,9 +1347,9 @@ int cmd_status(int argc, const char **argv, const\n> > > char *prefix)\n> > > \t N_(\"show status concisely\"), STATUS_FORMAT_SHORT),\n> > > OPT_BOOL('b', \"branch\", &s.show_branch,\n> > > \t\t\t N_(\"show branch information\")),\n> > > -\t\tOPT_SET_INT(0, \"porcelain\", &status_format,\n> > > -\t\t\t N_(\"machine-readable output\"),\n> > > -\t\t\t STATUS_FORMAT_PORCELAIN),\n> > > +\t\t{ OPTION_CALLBACK, 0, \"porcelain\", &status_format,\n> > > +\t\t N_(\"version\"), N_(\"machine-readable output\"),\n> > > +\t\t PARSE_OPT_OPTARG, opt_parse_porcelain },\n> >\n> > How about using a COUNTUP here instead? We could then set the status\n> > format afterwards, like this:\n> >\n> > if (porcelain == 0)\n> > \tstatus_format = STATUS_FORMAT_UNSPECIFIED;\n> > else {\n> > status_format = STATUS_FORMAT_PORCELAIN;\n> > if (porcelain > 1)\n> > warning(\"No porcelain v%d; falling back to v1\",\n> > \t\t\tporcelain);\n> > }\n> >\n> \n> Maybe I misread the COUNTUP docs, but it looked like it would\n> allow \"--porcelain --porcelain\", but not \"--porcelain=2\".\n\nWhoops, you're right. It is *I* who misread the code (I did not bother\nlooking for the docs ;-))\n\nStill, I would prefer to avoid that callback. IOW something like\n\n\t{ OPTION_INTEGER, 0, \"porcelain\", &status_format,\n\t N_(\"version\"), N_(\"machine-readable output\"),\n\t PARSE_OPT_OPTARG, NULL, 1 },\n\nfollowed by the if () outlined above.\n\nCiao,\nJohannes\n"},{"id":"291922","messageId":"alpine.DEB.2.20.1607211745050.14111@virtualbox","threadId":"42883","inReplyTo":"20160720205411.GB578@sigill.intra.peff.net","subject":"Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T15:46:43Z","receivedAt":"2016-07-21T15:47:55Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff & Jeff ;-)\n\nOn Wed, 20 Jul 2016, Jeff King wrote:\n\n> On Wed, Jul 20, 2016 at 02:20:24PM -0400, Jeff Hostetler wrote:\n> \n> > > IIRC, it happens when HEAD points to a broken ref. So something like:\n> > > \n> > > git init\n> > > echo broken >.git/refs/heads/master\n> > > \n> > > would cause resolving HEAD to return NULL.\n> > \n> > That worked and I see \"(unknown)\".\n> > \n> > This is a bit of a nit, but is there a value we'd like\n> > to see there, such as \"(unknown)\" or \"(broken)\" or \"(missing)\"\n> > in that case? (And make it clear that this is a different\n> > case from \"(detached)\".)\n> > \n> > I'm thinking it would be nicer to always have a field\n> > there for parsing.\n> \n> My gut feeling is to err on the side of being vague, like \"unknown\".\n> This is something that _shouldn't_ ever happen, and if it does, it could\n> be a broken on-disk ref, a transient syscall error, or some other\n> weirdness. I don't think we need to get too specific in this context\n> (we'll likely have said something else useful on stderr already, I\n> think).\n\nFWIW I think \"unknown\" is a nice conservative way to shrug Git's\nshoulders.\n\nWhen we call `git status --porcelain=v2` and read \"unknown\", we could\nalways try to find out more using additional low-level tools and/or disk\naccess: this is such a rare case that it does not *really* matter all that\nmuch.\n\nCiao,\nDscho\n"},{"id":"291923","messageId":"alpine.DEB.2.20.1607211749590.14111@virtualbox","threadId":"42883","inReplyTo":"CANQwDwfWW4Fn7KBHrNv2nfvGHMrPQ4jGpz6wGAEGg3edn-ztfg@mail.gmail.com","subject":"Re: [PATCH v1 6/6] Unit tests for V2 porcelain status","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T15:54:10Z","receivedAt":"2016-07-21T15:55:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 20 Jul 2016, Jakub Narębski wrote:\n\n> On 20 July 2016 at 17:47, Jeff Hostetler wrote:\n> > On 07/20/2016 11:30 AM, Jakub Narębski wrote:\n> >> W dniu 2016-07-20 o 00:10, Jeff Hostetler pisze:\n> >>>\n> >>> +test_expect_success pre_initial_commit_0 '\n> >>> + printf \"## branch: (initial) master\\n\" >expected &&\n> >>> + printf \"?? actual\\n\" >>expected &&\n> >>> + printf \"?? dir1/\\n\" >>expected &&\n> >>> + printf \"?? expected\\n\" >>expected &&\n> >>> + printf \"?? file_x\\n\" >>expected &&\n> >>> + printf \"?? file_y\\n\" >>expected &&\n> >>> + printf \"?? file_z\\n\" >>expected &&\n> >>\n> >> Why not use heredoc syntax (cat <<\\EOF), or prepare a file\n> >> with expected output in the testsuite?\n> >\n> > The tests involving renames needed to embed a tab character\n> > in the output and hiding a tab character in a heredoc seemed\n> > error prone. So to be consistent I made them all printf-style.\n> \n> Ah, so that's the case for series of printf. I think in some other\n> cases the Git testsuite simply uses HT variable for the TAB\n> character.\n\nYeah, it would be more pleasant to read\n\n\techo >expected <<-EOF\n\t## branch: (initial) master\n\t?? actual\n\t?? dir1/\n\t?? expected\n\t?? file_x\n\t?? file_y\n\t?? file_z\n\tEOF\n\nAnd it is also easy to use $HT in there (unless you want to use <<-\\EOF).\n\nActually, even if you want to use \\EOF, you can easily use `sed` to\nexpand, say, \"Q\" to tabs, such as was done here:\n\n\thttps://github.com/git/git/blob/v2.9.2/t/t4213-log-tabexpand.sh#L88-L92\n\nCiao,\nDscho"},{"id":"291924","messageId":"alpine.DEB.2.20.1607211800080.14111@virtualbox","threadId":"42883","inReplyTo":"20160720205753.GC578@sigill.intra.peff.net","subject":"Re: [PATCH v1 0/6] Porcelain Status V2","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-07-21T16:02:13Z","receivedAt":"2016-07-21T16:03:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Peff & Jeff ;-)\n\nOn Wed, 20 Jul 2016, Jeff King wrote:\n\n> On Wed, Jul 20, 2016 at 03:27:45PM -0400, Jeff Hostetler wrote:\n> \n> > > A totally reasonable response is \"haha no. Please stop moving the\n> > > goalposts\". I just wanted to throw it out there as an option (and in\n> > > case you are interested, to let you think about it before any more work\n> > > goes into this direction).\n> > \n> > haha no.... :-)\n> > \n> > Short term, I'd rather nail down what I have now (both content-wise\n> > and format-wise) and see how we like it. And have a follow-up task\n> > to look at the --state header we spoke of earlier. And save the JSON\n> > version as an independent task for later.\n> > \n> > I understand the motivation for a JSON option (and have thought\n> > about it before) but I think it ought to be kept separate.\n> > At a higher-level, it seems like a JSON option would be an\n> > opportunity to start a project-wide conversation about formats,\n> > consistency, plumbing, and etc. A top-down conversation if you\n> > will about which commands will/won't get enhanced, legacy cruft\n> > that would not need to be converted, JSON style and naming and\n> > consistency issues, current best practices in the node/whatever\n> > community, and etc. I could be wrong, but this feels like a\n> > top-down feature conversation in a wider audience.\n> \n> I agree with everything you've said here.\n> \n> If we add JSON, we'd want to do it everywhere: lists of commits, lists\n> of refs, status output, etc. I mentioned that somebody had asked me\n> about it recently; they are working on a git client and finding that\n> libgit2 is not serving their needs well, so they'd like to shell out to\n> git more, and wanted to have a standard way to get the data back in.\n\nYeah, if we add JSON, we would want to add it everywhere. But we would\nwant to add that incrementally; otherwise it would be too humongous a\ntask.\n\nAnd I think a good way forward was already suggested elsewhere in this\nthread by Jeff: --porcelain=1j, or --porcelain=json (and maybe later\njson-v2, json-v3, etc).\n\nCiao,\nDscho\n"},{"id":"291931","messageId":"57911B5B.9090604@jeffhostetler.com","threadId":"42883","inReplyTo":"xmqqh9bkc7xx.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-21T18:58:35Z","receivedAt":"2016-07-21T19:00:57Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/20/2016 05:31 PM, Junio C Hamano wrote:\n\n> The code seems to assume that d->porcelain_v2.* fields are\n> initialized earlier in the callchain to reasonable values\n> (e.g. STATUS_ADDED case does not clear .mode_head to \"missing\"); I\n> am not sure if that is easier to read or fill in all the values\n> explicitly in this function.\n\nThe structure is initially zeroed when it allocated.\nI'm only setting values for the side the we know are\nactually defined (such as in an ADD we know there is\nno head values). I'll look at making this more clear.\n\n>> +}\n>> +\n>> +/* Copy info for both sides of an index-vs-worktree change\n>> + * into the very verbose porcelain data.\n>> + */\n>> +static void porcelain_v2_changed_entry(\n>> +\tstruct wt_status_change_data *d,\n>> +\tconst struct diff_filepair *p)\n>> +{\n>> +\tswitch (p->status) {\n>> +\tcase DIFF_STATUS_ADDED:\n>> +\t\td->porcelain_v2.mode_worktree = p->two->mode;\n>> +\t\t/* don't bother with worktree sha, since it is almost always zero. */\n>> +\t\tbreak;\n>> +\n>> +\tcase DIFF_STATUS_DELETED:\n>> +\t\td->porcelain_v2.mode_index = p->one->mode;\n>> +\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n>> +\t\tbreak;\n>> +\n>> +\tcase DIFF_STATUS_COPIED:\n>> +\tcase DIFF_STATUS_MODIFIED:\n>> +\tcase DIFF_STATUS_RENAMED:\n>\n> Can RENAMED or COPIED happen here?\n\nIf we don't report renamed or copied for unstaged changes, then no.\n\n\n> The code would be simpler and we'll catch bugs more uniformly if\n> \"collect\" phase is made oblivious to s->status_format, I would\n> suspect, by making unconditional call to porcelain_v2_*_entry()\n> functions from all of these \"collect\" functions.\n\nok. will do.\n\n>\n>> +/* Convert various submodule status values into a\n>> + * string of characters in the buffer provided.\n>> + */\n>> +static void wt_porcelain_v2_submodule_state(\n>> +\tstruct wt_status_change_data *d,\n>> +\tchar sub[5])\n>> +{\n>> +\tint k = 0;\n>> +\n>> +\tif (S_ISGITLINK(d->porcelain_v2.mode_head) ||\n>> +\t\tS_ISGITLINK(d->porcelain_v2.mode_index) ||\n>> +\t\tS_ISGITLINK(d->porcelain_v2.mode_worktree)) {\n>> +\t\t/* We have a submodule */\n>> +\t\tsub[k++] = 'S';\n>> +\n>> +\t\t/* Sub-flags for each type of dirt */\n>> +\t\tif (d->new_submodule_commits)\n>> +\t\t\tsub[k++] = 'C';\n>> +\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED)\n>> +\t\t\tsub[k++] = 'M';\n>> +\t\tif (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED)\n>> +\t\t\tsub[k++] = 'U';\n>> +\t} else {\n>> +\t\t/* Not a submodule */\n>> +\t\tsub[k++] = 'N';\n>> +\t}\n>> +\n>> +\tsub[k] = 0;\n>> +}\n>\n> The above seems to do \"up to 5\"; if these come early in the output,\n> it may instead be easier to read if \"fixed 5 columns, many of them\n> may be SP, and some may be C or M or U\", which would align on fixed\n> column terminal people use for coding.\n\nI debated fixed 4 columns vs a variable width token.\nThe latter seemed easier to parse, but does look more ugly\non the terminal. I'll revisit this. Perhaps make it fixed\n4 columns with a '.' instead of SPACE, so both parsing\ntechniques work.\n\n>\n>> +/* Various fix-up steps before we start printing an item.\n>> + */\n>> +static void wt_porcelain_v2_fix_up_status(\n>> +\tstruct string_list_item *it,\n>> +\tstruct wt_status *s)\n>> +{\n>> +\tstruct wt_status_change_data *d = it->util;\n>> +\n>> +\tif (!d->index_status) {\n>> +\t\tif (d->worktree_status == DIFF_STATUS_MODIFIED ||\n>> +\t\t\td->worktree_status == DIFF_STATUS_DELETED) {\n>> +\t\t\t/* X=' ' Y=[MD]\n>> +\t\t\t * The item did not change in head-vs-index scan so the head\n>> +\t\t\t * column was never set. (The index column was set during the\n>> +\t\t\t * index-vs-worktree scan.)\n>> +\t\t\t * Force set the head column to make the output complete.\n>> +\t\t\t */\n>> +\t\t\td->porcelain_v2.mode_head = d->porcelain_v2.mode_index;\n>> +\t\t\thashcpy(d->porcelain_v2.sha1_head, d->porcelain_v2.sha1_index);\n>> +\t\t}\n>\n> Can there be \"else\" clause for this inner \"if\"? When\n> d->index_status is not set, but worktree_status is not one of these\n> two, what does it indicate? The same for the next one.\n\nFor a normal entry (not unmerged) when X is ' ', Y can only\nbe 'M' or 'D'. The only other value for Y is ' ', but then if\nboth were ' ' the entry would not be in the list. So I think\nwe're OK, but I'll document that here. And below.\n\n\n>> +/*\n>> + * Define a single format for tracked entries. This includes:\n>> + * normal changes, rename changes, and unmerged changes.\n>> + *\n>> + * The meanings of modes_[abcd] and sha_[abc] depends on the\n>> + * change type, but are always present.\n>> + *\n>> + * Path(s) are C-Quoted if necessary. Current path is ALWAYS\n>> + * first. The rename source path is only present when necessary.\n>> + * A single TAB separates them (because paths can contain spaces\n>> + * and C-Quoting converts actual tabs in pathnames to a C escape\n>> + * sequence).\n>> + */\n>> +static void wt_porcelain_v2_print_tracked_entry(\n>> +\tFILE *fp,\n>> +\tchar x_staged,\n>> +\tchar y_unstaged,\n>> +\tconst char *submodule,\n>> +\tint mode_a,\n>> +\tint mode_b,\n>> +\tint mode_c,\n>> +\tint mode_d,\n>> +\tconst unsigned char sha_a[GIT_SHA1_RAWSZ],\n>> +\tconst unsigned char sha_b[GIT_SHA1_RAWSZ],\n>> +\tconst unsigned char sha_c[GIT_SHA1_RAWSZ],\n>\n> x, y, a, b, c, d are all mystery. When we see a/b/c, they typically\n> mean stage#1, stage#2 and stage#3, but that does not seem to be it.\n> When we see a/b, they typically mean old and new, but that does not\n> seem to be the case here, either.\n\nI took X and Y from the man-page as the names of first 2 columns\nof the existing format.\n>\n> If you are saying in the above comment, which curiously is formatted\n> correctly unlike many other multi-line comments we see in this patch\n> ;-),\n\nI read Linus' recent post on the subject mid-way thru this effort. :-)\n\n > that the meaning of these are not fixed but the function is to\n> always show 2 things, then four things in the fixed order without\n> knowing what they mean, it would be easier to understand if you made\n> them into an array, i.e.e.g. instead of having x_staged and\n> y_unstaged, have \"char prefix[2]\" or something that tells the reader\n> that whatever meaning these have, these come early in the output.\n\nI'll revisit this. For normal entries (modified and/or renamed) I\nshow the head, index, worktree modes and the head and index SHAs\nand the last mode and SHA fields are zero. For unmerged entries,\nI show the stage 1, 2, 3 and worktree modes and the stage 1,2,3\nSHAs. This gives us the same number of fields for parsing. An\nearlier version had 2 different line formats. Would it be better to\nhave a different initial character/token (like the \":\" vs \"::\" prefix\nin diff-index) ?\n\n\n>> +\tx_staged = d->index_status ? d->index_status : '.';\n>> +\ty_unstaged = d->worktree_status ? d->worktree_status : '.';\n>\n> I see a curious and misleading use of the word \"staged\" around\n> here. If these two are index-status and worktree-status (with\n> stand-in letter '.' when they are not available), why not call them\n> index-status and worktree-status instead?\n> ...\n>> +\tchar x_us = 'U', y_them = 'U';\n>\n> I really do not want to see these X_ and Y_ prefixes; they do not\n> convey any meaning; if X always corresponds to ours while Y always\n> corresponds to theirs, then they are even redundant.\n\nI was trying to preserve the X and Y link back to the man-page, but\nalso give meaning to the fields. I'll revisit.\n\n>> +\tmemset(stages, 0, sizeof(stages));\n>> +\tpos = cache_name_pos(it->string, strlen(it->string));\n>> +\tassert(pos < 0);\n>> +\tpos = -pos-1;\n>> +\twhile (pos < active_nr) {\n>> +\t\tce = active_cache[pos++];\n>> +\t\tstage = ce_stage(ce);\n>> +\t\tif (strcmp(ce->name, it->string) || !stage)\n>> +\t\t\tbreak;\n>> +\t\tstages[stage - 1].mode = ce->ce_mode;\n>> +\t\thashcpy(stages[stage - 1].sha1, ce->sha1);\n>> +\t}\n>\n> You did assert(pos < 0) to make sure that the path the caller told\n> you is unmerged indeed is unmerged, which is a very good check. In\n> the same spirit, you would want to make sure that you got at least\n> one result from the above loop, to diagnose a bug where the path did\n> not even exist at all in the index.\n\nWould that be possible for a conflict/unmerged entry?\nI can add an assert for that just to be safe.\n\n> Also, it is possible to have more than one stage#1 entries in the\n> index (stage#2 can only be one; I think stage#3 can theoretically\n> have more than one, but in practice, even merge-octopus does not do\n> that). The above loop gives us the \"last one wins\" semantics to\n> make sure what you grab from the index always fit in stages[] array,\n> which is not wrong per-se, but that design decision may want to be\n> documented in a comment nearby for people to work on enhancing it in\n> the future.\n\nInteresting, more than 1 entry in a single stage. I hadn't considered\nthat. I think I pretty much stole the essence of this loop from\nelsewhere in the file, so yes I should be consistent with elsewhere\nin the code. I'll update the comments. Good catch!\n\nThanks for all your time on this!\nJeff\n\n"},{"id":"291932","messageId":"57911C65.2050601@jeffhostetler.com","threadId":"42883","inReplyTo":"alpine.DEB.2.20.1607211745050.14111@virtualbox","subject":"Re: [PATCH v1 4/6] Expanded branch header for Porcelain Status V2","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2016-07-21T19:03:01Z","receivedAt":"2016-07-21T19:05:22Z","isPatch":true,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 07/21/2016 11:46 AM, Johannes Schindelin wrote:\n> On Wed, 20 Jul 2016, Jeff King wrote:\n>\n>> On Wed, Jul 20, 2016 at 02:20:24PM -0400, Jeff Hostetler wrote:\n>>\n>>>> IIRC, it happens when HEAD points to a broken ref. So something like:\n>>>>\n>>>> git init\n>>>> echo broken >.git/refs/heads/master\n>>>>\n>>>> would cause resolving HEAD to return NULL.\n>>>\n>>> That worked and I see \"(unknown)\".\n>>>\n>>> This is a bit of a nit, but is there a value we'd like\n>>> to see there, such as \"(unknown)\" or \"(broken)\" or \"(missing)\"\n>>> in that case? (And make it clear that this is a different\n>>> case from \"(detached)\".)\n>>>\n>>> I'm thinking it would be nicer to always have a field\n>>> there for parsing.\n>>\n>> My gut feeling is to err on the side of being vague, like \"unknown\".\n>> This is something that _shouldn't_ ever happen, and if it does, it could\n>> be a broken on-disk ref, a transient syscall error, or some other\n>> weirdness. I don't think we need to get too specific in this context\n>> (we'll likely have said something else useful on stderr already, I\n>> think).\n>\n> FWIW I think \"unknown\" is a nice conservative way to shrug Git's\n> shoulders.\n>\n> When we call `git status --porcelain=v2` and read \"unknown\", we could\n> always try to find out more using additional low-level tools and/or disk\n> access: this is such a rare case that it does not *really* matter all that\n> much.\n\nyes, this case causes even rev-parse fits. I'l make it return a\nknown quantity so that users don't have to deal with stuff like:\n\n$ more .git/HEAD\nref: refs/heads/foo\n$ more .git/refs/heads/foo\nbrokwn\n$ git rev-parse HEAD\nHEAD\nfatal: ambiguous argument 'HEAD': unknown revision or path not in the \nworking tree.\nUse '--' to separate paths from revisions, like this:\n'git [...] -- [...]'\n$ git rev-parse HEAD --\nfatal: bad revision 'HEAD'\n$\n\n\nJeff\n"},{"id":"291977","messageId":"xmqqinvxboa1.fsf@gitster.mtv.corp.google.com","threadId":"42883","inReplyTo":"57911B5B.9090604@jeffhostetler.com","subject":"Re: [PATCH v1 3/6] Per-file output for Porcelain Status V2","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-07-22T17:01:10Z","receivedAt":"2016-07-22T17:01:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff Hostetler writes:\n\n>>> +\tcase DIFF_STATUS_DELETED:\n>>> +\t\td->porcelain_v2.mode_index = p->one->mode;\n>>> +\t\thashcpy(d->porcelain_v2.sha1_index, p->one->sha1);\n>>> +\t\tbreak;\n>>> +\n>>> +\tcase DIFF_STATUS_COPIED:\n>>> +\tcase DIFF_STATUS_MODIFIED:\n>>> +\tcase DIFF_STATUS_RENAMED:\n>>\n>> Can RENAMED or COPIED happen here?\n>\n> If we don't report renamed or copied for unstaged changes, then no.\n\nThe question was \"do we report such cases\"?\n\n>>> +\tif (!d->index_status) {\n>>> +\t\tif (d->worktree_status == DIFF_STATUS_MODIFIED ||\n>>> +\t\t\td->worktree_status == DIFF_STATUS_DELETED) {\n>>> +\t\t\t/* X=' ' Y=[MD]\n>>> +\t\t\t * The item did not change in head-vs-index scan so the head\n>>> +\t\t\t * column was never set. (The index column was set during the\n>>> +\t\t\t * index-vs-worktree scan.)\n>>> +\t\t\t * Force set the head column to make the output complete.\n>>> +\t\t\t */\n>>> +\t\t\td->porcelain_v2.mode_head = d->porcelain_v2.mode_index;\n>>> +\t\t\thashcpy(d->porcelain_v2.sha1_head, d->porcelain_v2.sha1_index);\n>>> +\t\t}\n>>\n>> Can there be \"else\" clause for this inner \"if\"? When\n>> d->index_status is not set, but worktree_status is not one of these\n>> two, what does it indicate? The same for the next one.\n>\n> For a normal entry (not unmerged) when X is ' ', Y can only\n> be 'M' or 'D'. The only other value for Y is ' ', but then if\n> both were ' ' the entry would not be in the list. So I think\n> we're OK, but I'll document that here. And below.\n\nGood.\n\n>>> +\tmemset(stages, 0, sizeof(stages));\n>>> +\tpos = cache_name_pos(it->string, strlen(it->string));\n>>> +\tassert(pos < 0);\n>>> +\tpos = -pos-1;\n>>> +\twhile (pos < active_nr) {\n>>> +\t\tce = active_cache[pos++];\n>>> +\t\tstage = ce_stage(ce);\n>>> +\t\tif (strcmp(ce->name, it->string) || !stage)\n>>> +\t\t\tbreak;\n>>> +\t\tstages[stage - 1].mode = ce->ce_mode;\n>>> +\t\thashcpy(stages[stage - 1].sha1, ce->sha1);\n>>> +\t}\n>>\n>> You did assert(pos < 0) to make sure that the path the caller told\n>> you is unmerged indeed is unmerged, which is a very good check. In\n>> the same spirit, you would want to make sure that you got at least\n>> one result from the above loop, to diagnose a bug where the path did\n>> not even exist at all in the index.\n>\n> Would that be possible for a conflict/unmerged entry?\n\nIt is possible to exactly the same degree as it is possible you\nwould get !(pos < 0) answer from cache_name_pos() here, I would\nthink. The assert() you have up above is protecting us from either\na broken caller to this function that gives an it that points at a\nmerged path (in which case the assert is violated) or a breakage in\ncache_name_pos(). Making sure the loop finds at least one unmerged\nentry protects us from similar kind of breakage that violates the\nexpectation this code has from the other parts of the code.\n\n\nThanks.\n"}]}