{"thread":{"id":"17688","subject":"[RFC/PATCH] shortstatus v1","startedAt":"2009-02-10T00:51:07Z","lastAt":"2009-02-12T00:49:21Z","messageCount":25,"participants":["Tuncer Ayaz","Junio C Hamano","Sitaram Chamarty","Johannes Schindelin","Jeff King","Michael J Gruber","Nanako Shiraishi"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"103937","messageId":"1234227067-56666-1-git-send-email-tuncer.ayaz@gmail.com","threadId":"17688","inReplyTo":null,"subject":"[RFC/PATCH] shortstatus v1","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-02-10T00:51:07Z","receivedAt":"2009-02-10T00:51:07Z","isPatch":true,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"As discussed recently I started taking Junio's shortstatus patch\nfrom October 25th 2008 and integrated it into current master.\n\nThis revision does work as advertised by Junio and v0 also did.\n\nThis revision also implements an experimental --mini param to\nshortstatus which prints the following:\n anything modified -> * (working)\n anything added -> + (working)\n anything untracked/unknown -> ? (not yet implemented)\n\nSo if you have a repo where one file is modified,\na new file is added and an unknown file exists and\nis not ignored 'shortstatus --mini' prints:\n+*?.\n\nThis is really useful for enhancing a Git enabled\nshell prompt with small but important information.\n\nRight now this is basically Junio's shortstatus\nfrom Oct 25th 2008 with no substantial change\nexcept a line or two.\n\nAdding git 'shortstatus --mini' to PS1 is not noticeable or 1sec\nmaximum in my tree. As a worst case it takes 10secs in a clone\nof WebKit.git.\n\nTODO:\n - print ? if untracked/unknown found\n - maybe implement git-ministatus instead of git-shortstatus --mini\n - as Junio mentioned maybe we should not print the index_score\n - peer review (mini clause/mode, especially the switch-case)\n - peer review rest of the patch\n\nSigned-off-by: Tuncer Ayaz \n---\n.gitignore | 1 \n Makefile | 1 \n builtin-commit.c | 92 +++++++++++++++++++++++\n builtin-revert.c | 1 \n builtin.h | 1 \n git.c | 1 \n wt-status.c | 213 +++++++++++++++++++++++++++++++++++++++++++------------\n wt-status.h | 9 ++\n 8 files changed, 273 insertions(+), 46 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 13311f1..055eb54 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -115,6 +115,7 @@ git-send-pack\n git-sh-setup\n git-shell\n git-shortlog\n+git-shortstatus\n git-show\n git-show-branch\n git-show-index\ndiff --git a/Makefile b/Makefile\nindex 27b9569..a0ca137 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -330,6 +330,7 @@ BUILT_INS += git-repo-config$X\n BUILT_INS += git-show$X\n BUILT_INS += git-stage$X\n BUILT_INS += git-status$X\n+BUILT_INS += git-shortstatus$X\n BUILT_INS += git-whatchanged$X\n \n # what 'all' will build and 'install' will install, in gitexecdir\ndiff --git a/builtin-commit.c b/builtin-commit.c\nindex d6a3a62..9267d26 100644\n--- a/builtin-commit.c\n+++ b/builtin-commit.c\n@@ -14,6 +14,7 @@\n #include \"diffcore.h\"\n #include \"commit.h\"\n #include \"revision.h\"\n+#include \"string-list.h\"\n #include \"wt-status.h\"\n #include \"run-command.h\"\n #include \"refs.h\"\n@@ -21,7 +22,6 @@\n #include \"strbuf.h\"\n #include \"utf8.h\"\n #include \"parse-options.h\"\n-#include \"string-list.h\"\n #include \"rerere.h\"\n #include \"unpack-trees.h\"\n \n@@ -35,6 +35,11 @@ static const char * const builtin_status_usage[] = {\n \tNULL\n };\n \n+static const char * const builtin_shortstatus_usage[] = {\n+\t\"git shortstatus [options] [--] ...\",\n+\tNULL\n+};\n+\n static unsigned char head_sha1[20], merge_head_sha1[20];\n static char *use_message_buffer;\n static const char commit_editmsg[] = \"COMMIT_EDITMSG\";\n@@ -51,7 +56,7 @@ static const char *template_file;\n static char *edit_message, *use_message;\n static char *author_name, *author_email, *author_date;\n static int all, edit_flag, also, interactive, only, amend, signoff;\n-static int quiet, verbose, no_verify, allow_empty;\n+static int quiet, verbose, no_verify, allow_empty, mini;\n static char *untracked_files_arg;\n /*\n * The default commit message cleanup mode will remove the lines\n@@ -107,6 +112,7 @@ static struct option builtin_commit_options[] = {\n \t{ OPTION_STRING, 'u', \"untracked-files\", &untracked_files_arg, \"mode\", \"show untracked files, optional modes: all, normal, no. (Default: all)\", PARSE_OPT_OPTARG, NULL, (intptr_t)\"all\" },\n \tOPT_BOOLEAN(0, \"allow-empty\", &allow_empty, \"ok to record an empty change\"),\n \tOPT_STRING(0, \"cleanup\", &cleanup_arg, \"default\", \"how to strip spaces and #comments from message\"),\n+\tOPT_BOOLEAN(0, \"mini\", &mini, \"print mini shortstatus\"),\n \n \tOPT_END()\n };\n@@ -821,6 +827,88 @@ static int parse_and_validate_options(int argc, const char *argv[],\n \treturn argc;\n }\n \n+int cmd_shortstatus(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct wt_status s;\n+\tint i;\n+\tint c, a, u;\n+\n+\tc = a = u = 0;\n+\n+\targc = parse_and_validate_options(argc, argv, builtin_shortstatus_usage, prefix);\n+\tread_cache();\n+\trefresh_cache(REFRESH_QUIET);\n+\twt_status_prepare(&s);\n+\twt_status_collect_changes(&s);\n+\tif (mini) {\n+\t\tfor (i = 0; i < s.change.nr; i++) {\n+\t\t\tstruct wt_status_change_data *d;\n+\t\t\tstruct string_list_item *it;\n+\n+\t\t\tit = &(s.change.items[i]);\n+\t\t\td = it->util;\n+\t\t\tswitch (d->index_status) {\n+\t\t\t\tcase DIFF_STATUS_ADDED:\n+\t\t\t\t\ta = 1;\n+\t\t\t\t\tbreak;\n+\t\t\t\tcase 0:\n+\t\t\t\tcase DIFF_STATUS_COPIED:\n+\t\t\t\tcase DIFF_STATUS_DELETED:\n+\t\t\t\tcase DIFF_STATUS_MODIFIED:\n+\t\t\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\t\tcase DIFF_STATUS_TYPE_CHANGED:\n+\t\t\t\t\tc = 1;\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\tcase DIFF_STATUS_UNKNOWN:\n+\t\t\t\tcase DIFF_STATUS_UNMERGED:\n+\t\t\t\t\tu = 1;\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t\tif (c)\n+\t\t\tprintf(\"*\");\n+\t\tif (a)\n+\t\t\tprintf(\"+\");\n+\t\tif (u)\n+\t\t\tprintf(\"?\");\n+\t} else {\n+\t\tfor (i = 0; i < s.change.nr; i++) {\n+\t\t\tstruct wt_status_change_data *d;\n+\t\t\tstruct string_list_item *it;\n+\t\t\tchar pfx[1 + 3 + 1 + 1];\n+\n+\t\t\tit = &(s.change.items[i]);\n+\t\t\td = it->util;\n+\t\t\tswitch (d->index_status) {\n+\t\t\t\tcase DIFF_STATUS_COPIED:\n+\t\t\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\t\t\tsprintf(pfx, \"%c%3d\",\n+\t\t\t\t\t\t\td->index_status,\n+\t\t\t\t\t\t\t(int)(d->index_score * 100 / MAX_SCORE));\n+\t\t\t\t\tbreak;\n+\t\t\t\tcase 0:\n+\t\t\t\t\tmemcpy(pfx, \"\t\", 4);\n+\t\t\t\t\tbreak;\n+\t\t\t\tdefault:\n+\t\t\t\t\tsprintf(pfx, \"%c\t \", d->index_status);\n+\t\t\t\t\tbreak;\n+\t\t\t}\n+\t\t\tif (!d->worktree_status)\n+\t\t\t\tpfx[4] = ' ';\n+\t\t\telse\n+\t\t\t\tpfx[4] = d->worktree_status;\n+\t\t\tpfx[5] = '\\0';\n+\t\t\tprintf(\"%s \", pfx);\n+\t\t\tif (d->head_path)\n+\t\t\t\tprintf(\"%s -> \", d->head_path);\n+\t\t\tprintf(\"%s\\n\", it->string);\n+\t\t}\n+\n+\t}\n+\treturn 0;\n+}\n+\n int cmd_status(int argc, const char **argv, const char *prefix)\n {\n \tconst char *index_file;\ndiff --git a/builtin-revert.c b/builtin-revert.c\nindex d48313c..7dd7646 100644\n--- a/builtin-revert.c\n+++ b/builtin-revert.c\n@@ -3,6 +3,7 @@\n #include \"object.h\"\n #include \"commit.h\"\n #include \"tag.h\"\n+#include \"string-list.h\"\n #include \"wt-status.h\"\n #include \"run-command.h\"\n #include \"exec_cmd.h\"\ndiff --git a/builtin.h b/builtin.h\nindex 1495cf6..f054fc7 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -94,6 +94,7 @@ extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\n extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_status(int argc, const char **argv, const char *prefix);\n+extern int cmd_shortstatus(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\n extern int cmd_tag(int argc, const char **argv, const char *prefix);\ndiff --git a/git.c b/git.c\nindex c2b181e..4c0fa44 100644\n--- a/git.c\n+++ b/git.c\n@@ -344,6 +344,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"rm\", cmd_rm, RUN_SETUP },\n \t\t{ \"send-pack\", cmd_send_pack, RUN_SETUP },\n \t\t{ \"shortlog\", cmd_shortlog, USE_PAGER },\n+\t\t{ \"shortstatus\", cmd_shortstatus, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t\t{ \"show\", cmd_show, RUN_SETUP | USE_PAGER },\n \t\t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\ndiff --git a/wt-status.c b/wt-status.c\nindex 96ff2f8..18042dc 100644\n--- a/wt-status.c\n+++ b/wt-status.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"string-list.h\"\n #include \"wt-status.h\"\n #include \"color.h\"\n #include \"object.h\"\n@@ -56,6 +57,7 @@ void wt_status_prepare(struct wt_status *s)\n \ts->reference = \"HEAD\";\n \ts->fp = stdout;\n \ts->index_file = get_index_file();\n+\ts->change.strdup_strings = 1;\n }\n \n static void wt_status_print_cached_header(struct wt_status *s)\n@@ -98,18 +100,23 @@ static void wt_status_print_trailer(struct wt_status *s)\n \n #define quote_path quote_path_relative\n \n-static void wt_status_print_filepair(struct wt_status *s,\n-\t\t\t\t int t, struct diff_filepair *p)\n+static void wt_status_print_change_data(struct wt_status *s,\n+\t\t\t\t\t\tint t,\n+\t\t\t\t\t\tint status,\n+\t\t\t\t\t\tchar *one_name,\n+\t\t\t\t\t\tchar *two_name,\n+\t\t\t\t\t\tint score)\n {\n \tconst char *c = color(t);\n \tconst char *one, *two;\n \tstruct strbuf onebuf = STRBUF_INIT, twobuf = STRBUF_INIT;\n \n-\tone = quote_path(p->one->path, -1, &onebuf, s->prefix);\n-\ttwo = quote_path(p->two->path, -1, &twobuf, s->prefix);\n+\tone = quote_path(one_name, -1, &onebuf, s->prefix);\n+\ttwo = quote_path(two_name, -1, &twobuf, s->prefix);\n+\n \n \tcolor_fprintf(s->fp, color(WT_STATUS_HEADER), \"#\\t\");\n-\tswitch (p->status) {\n+\tswitch (status) {\n \tcase DIFF_STATUS_ADDED:\n \t\tcolor_fprintf(s->fp, c, \"new file: %s\", one);\n \t\tbreak;\n@@ -135,64 +142,88 @@ static void wt_status_print_filepair(struct wt_status *s,\n \t\tcolor_fprintf(s->fp, c, \"unmerged: %s\", one);\n \t\tbreak;\n \tdefault:\n-\t\tdie(\"bug: unhandled diff status %c\", p->status);\n+\t\tdie(\"bug: unhandled diff status %c\", status);\n \t}\n \tfprintf(s->fp, \"\\n\");\n \tstrbuf_release(&onebuf);\n \tstrbuf_release(&twobuf);\n }\n \n-static void wt_status_print_updated_cb(struct diff_queue_struct *q,\n-\t\tstruct diff_options *options,\n-\t\tvoid *data)\n+static void wt_status_collect_changed_cb(struct diff_queue_struct *q,\n+\t\t\t\t\t\t\tstruct diff_options *options,\n+\t\t\t\t\t\t\tvoid *data)\n {\n \tstruct wt_status *s = data;\n-\tint shown_header = 0;\n \tint i;\n+\n+\tif (!q->nr)\n+\t\treturn;\n+\ts->workdir_dirty = 1;\n \tfor (i = 0; i < q->nr; i++) {\n-\t\tif (q->queue[i]->status == 'U')\n-\t\t\tcontinue;\n-\t\tif (!shown_header) {\n-\t\t\twt_status_print_cached_header(s);\n-\t\t\ts->commitable = 1;\n-\t\t\tshown_header = 1;\n-\t\t}\n-\t\twt_status_print_filepair(s, WT_STATUS_UPDATED, q->queue[i]);\n+\t\tstruct diff_filepair *p;\n+\t\tstruct string_list_item *it;\n+\t\tstruct wt_status_change_data *d;\n+\n+\t\tp = q->queue[i];\n+\n+\t\td = xcalloc(1, sizeof(*d));\n+\t\td->worktree_status = p->status;\n+\t\tit = string_list_insert(p->one->path, &s->change);\n+\t\tit->util = d;\n \t}\n-\tif (shown_header)\n-\t\twt_status_print_trailer(s);\n }\n \n-static void wt_status_print_changed_cb(struct diff_queue_struct *q,\n- struct diff_options *options,\n- void *data)\n+static void wt_status_collect_updated_cb(struct diff_queue_struct *q,\n+\t\t\t\t\t\t\tstruct diff_options *options,\n+\t\t\t\t\t\t\tvoid *data)\n {\n \tstruct wt_status *s = data;\n \tint i;\n-\tif (q->nr) {\n-\t\tint has_deleted = 0;\n-\t\ts->workdir_dirty = 1;\n-\t\tfor (i = 0; i < q->nr; i++)\n-\t\t\tif (q->queue[i]->status == DIFF_STATUS_DELETED) {\n-\t\t\t\thas_deleted = 1;\n+\n+\tfor (i = 0; i < q->nr; i++) {\n+\t\tstruct diff_filepair *p;\n+\t\tstruct string_list_item *it;\n+\t\tstruct wt_status_change_data *d;\n+\n+\t\tp = q->queue[i];\n+\t\tit = string_list_insert(p->two->path, &s->change);\n+\t\td = it->util;\n+\t\tif (!d) {\n+\t\t\td = xcalloc(1, sizeof(*d));\n+\t\t\tit->util = d;\n+\t\t}\n+\t\td->index_status = p->status;\n+\t\tswitch (p->status) {\n+\t\t\tcase DIFF_STATUS_COPIED:\n+\t\t\tcase DIFF_STATUS_RENAMED:\n+\t\t\t\td->head_path = xstrdup(p->one->path);\n+\t\t\t\td->index_score = p->score;\n \t\t\t\tbreak;\n-\t\t\t}\n-\t\twt_status_print_dirty_header(s, has_deleted);\n+\t\t}\n \t}\n-\tfor (i = 0; i < q->nr; i++)\n-\t\twt_status_print_filepair(s, WT_STATUS_CHANGED, q->queue[i]);\n-\tif (q->nr)\n-\t\twt_status_print_trailer(s);\n }\n \n-static void wt_status_print_updated(struct wt_status *s)\n+static void wt_status_collect_changes_worktree(struct wt_status *s)\n {\n \tstruct rev_info rev;\n+\n+\tinit_revisions(&rev, NULL);\n+\tsetup_revisions(0, NULL, &rev, NULL);\n+\trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n+\trev.diffopt.format_callback = wt_status_collect_changed_cb;\n+\trev.diffopt.format_callback_data = s;\n+\trun_diff_files(&rev, 0);\n+}\n+\n+static void wt_status_collect_changes_index(struct wt_status *s)\n+{\n+\tstruct rev_info rev;\n+\n \tinit_revisions(&rev, NULL);\n \tsetup_revisions(0, NULL, &rev,\n \t\ts->is_initial ? EMPTY_TREE_SHA1_HEX : s->reference);\n \trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n-\trev.diffopt.format_callback = wt_status_print_updated_cb;\n+\trev.diffopt.format_callback = wt_status_collect_updated_cb;\n \trev.diffopt.format_callback_data = s;\n \trev.diffopt.detect_rename = 1;\n \trev.diffopt.rename_limit = 200;\n@@ -200,15 +231,107 @@ static void wt_status_print_updated(struct wt_status *s)\n \trun_diff_index(&rev, 1);\n }\n \n+static void wt_status_collect_changes_initial(struct wt_status *s)\n+{\n+\tint i;\n+\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct string_list_item *it;\n+\t\tstruct wt_status_change_data *d;\n+\n+\t\tit = string_list_insert(active_cache[i]->name, &s->change);\n+\t\td = it->util;\n+\t\tif (!d) {\n+\t\t\td = xcalloc(1, sizeof(*d));\n+\t\t\tit->util = d;\n+\t\t}\n+\t\td->index_status = DIFF_STATUS_ADDED;\n+\t}\n+}\n+\n+void wt_status_collect_changes(struct wt_status *s)\n+{\n+\twt_status_collect_changes_worktree(s);\n+\n+\tif (s->is_initial)\n+\t\twt_status_collect_changes_initial(s);\n+\telse\n+\t\twt_status_collect_changes_index(s);\n+}\n+\n+static void wt_status_print_updated(struct wt_status *s)\n+{\n+\tint shown_header = 0;\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+\t\tit = &(s->change.items[i]);\n+\t\td = it->util;\n+\t\tif (!d->index_status)\n+\t\t\tcontinue;\n+\t\tif (!shown_header) {\n+\t\t\twt_status_print_cached_header(s);\n+\t\t\ts->commitable = 1;\n+\t\t\tshown_header = 1;\n+\t\t}\n+\t\twt_status_print_change_data(s, WT_STATUS_UPDATED,\n+\t\t\t\td->index_status,\n+\t\t\t\td->head_path ? d->head_path : it->string,\n+\t\t\t\tit->string,\n+\t\t\t\td->index_score);\n+\t}\n+\tif (shown_header)\n+\t\twt_status_print_trailer(s);\n+}\n+\n+/*\n+ * -1 : has delete\n+ * 0 : no change\n+ * 1 : some change but no delete\n+ */\n+static int wt_status_check_worktree_changes(struct wt_status *s)\n+{\n+\tint i;\n+\tint changes = 0;\n+\n+\tfor (i = 0; i < s->change.nr; i++) {\n+\t\tstruct wt_status_change_data *d;\n+\t\td = s->change.items[i].util;\n+\t\tif (!d->worktree_status)\n+\t\t\tcontinue;\n+\t\tchanges = 1;\n+\t\tif (d->worktree_status == DIFF_STATUS_DELETED)\n+\t\t\treturn -1;\n+\t}\n+\treturn changes;\n+}\n+\n static void wt_status_print_changed(struct wt_status *s)\n {\n-\tstruct rev_info rev;\n-\tinit_revisions(&rev, \"\");\n-\tsetup_revisions(0, NULL, &rev, NULL);\n-\trev.diffopt.output_format |= DIFF_FORMAT_CALLBACK;\n-\trev.diffopt.format_callback = wt_status_print_changed_cb;\n-\trev.diffopt.format_callback_data = s;\n-\trun_diff_files(&rev, 0);\n+\tint i;\n+\tint worktree_changes = wt_status_check_worktree_changes(s);\n+\n+\tif (!worktree_changes)\n+\t\treturn;\n+\n+\twt_status_print_dirty_header(s, worktree_changes < 0);\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+\t\tit = &(s->change.items[i]);\n+\t\td = it->util;\n+\t\tif (!d->worktree_status)\n+\t\t\tcontinue;\n+\t\twt_status_print_change_data(s, WT_STATUS_CHANGED,\n+\t\t\t\td->worktree_status,\n+\t\t\t\tit->string,\n+\t\t\t\tit->string,\n+\t\t\t\t0);\n+\t}\n+\twt_status_print_trailer(s);\n }\n \n static void wt_status_print_submodule_summary(struct wt_status *s)\n@@ -338,6 +461,8 @@ void wt_status_print(struct wt_status *s)\n \t\t\twt_status_print_tracking(s);\n \t}\n \n+\twt_status_collect_changes(s);\n+\n \tif (s->is_initial) {\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"#\");\n \t\tcolor_fprintf_ln(s->fp, color(WT_STATUS_HEADER), \"# Initial commit\");\ndiff --git a/wt-status.h b/wt-status.h\nindex 78add09..00508c3 100644\n--- a/wt-status.h\n+++ b/wt-status.h\n@@ -18,6 +18,13 @@ enum untracked_status_type {\n };\n extern enum untracked_status_type show_untracked_files;\n \n+struct wt_status_change_data {\n+\tint worktree_status;\n+\tint index_status;\n+\tint index_score;\n+\tchar *head_path;\n+};\n+\n struct wt_status {\n \tint is_initial;\n \tchar *branch;\n@@ -33,6 +40,7 @@ struct wt_status {\n \tconst char *index_file;\n \tFILE *fp;\n \tconst char *prefix;\n+\tstruct string_list change;\n };\n \n int git_status_config(const char *var, const char *value, void *cb);\n@@ -40,5 +48,6 @@ extern int wt_status_use_color;\n extern int wt_status_relative_paths;\n void wt_status_prepare(struct wt_status *s);\n void wt_status_print(struct wt_status *s);\n+void wt_status_collect_changes(struct wt_status *s);\n \n #endif /* STATUS_H */\n"},{"id":"103942","messageId":"7vr627qd4p.fsf@gitster.siamese.dyndns.org","threadId":"17688","inReplyTo":"1234227067-56666-1-git-send-email-tuncer.ayaz@gmail.com","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T01:44:38Z","receivedAt":"2009-02-10T01:44:38Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tuncer Ayaz writes:\n\n> Adding git 'shortstatus --mini' to PS1 is not noticeable or 1sec\n> maximum in my tree. As a worst case it takes 10secs in a clone\n> of WebKit.git.\n\nFrankly, I think having to spend one second to add only one or two bits to\nPS1 is simply spending one second too much.\n\n> diff --git a/builtin-commit.c b/builtin-commit.c\n> index d6a3a62..9267d26 100644\n> --- a/builtin-commit.c\n> +++ b/builtin-commit.c\n> @@ -821,6 +827,88 @@ static int parse_and_validate_options(int argc, const char *argv[],\n> \treturn argc;\n> }\n> \n> +int cmd_shortstatus(int argc, const char **argv, const char *prefix)\n> +{\n> +\tstruct wt_status s;\n> +\tint i;\n> +\tint c, a, u;\n> +\n> +\tc = a = u = 0;\n> +\n> +\targc = parse_and_validate_options(argc, argv, builtin_shortstatus_usage, prefix);\n> +\tread_cache();\n> +\trefresh_cache(REFRESH_QUIET);\n> +\twt_status_prepare(&s);\n> +\twt_status_collect_changes(&s);\n> +\tif (mini) {\n> +\t\tfor (i = 0; i < s.change.nr; i++) {\n> +\t\t\tstruct wt_status_change_data *d;\n> +\t\t\tstruct string_list_item *it;\n> +\n> +\t\t\tit = &(s.change.items[i]);\n> +\t\t\td = it->util;\n> +\t\t\tswitch (d->index_status) {\n> +\t\t\t\tcase DIFF_STATUS_ADDED:\n> +\t\t\t\t\ta = 1;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tcase 0:\n> +\t\t\t\tcase DIFF_STATUS_COPIED:\n> +\t\t\t\tcase DIFF_STATUS_DELETED:\n> +\t\t\t\tcase DIFF_STATUS_MODIFIED:\n> +\t\t\t\tcase DIFF_STATUS_RENAMED:\n> +\t\t\t\tcase DIFF_STATUS_TYPE_CHANGED:\n> +\t\t\t\t\tc = 1;\n> +\t\t\t\t\tbreak;\n\nIf you at the end discard information by squashing renamed, copied,\ndeleted and modified into a single \"changed\" category, I do not think you\nwould want wt_status_collect_changes() to spend the cost of rename\ndetection in the first place. Sure, you can tell between \"git mv old new\"\nand \"git add new\", because you won't show \"+\" for \"new\" if you run rename\ndetection, but that is about the only thing I think you are getting.\n\nIs it worth extra 1 second (or 10 seconds)?\n\nWhat are you really trying to achieve? Do you want to see if you have any\nchange to the index since you checked out? Do you want to further tell\nthe user that the work tree has more changes that are not staged yet\n(which --mini does not seem to do)?\n\nDo you really need more than \"diff-index --cached --exit-code\" in your\n$PS1 code, and so why? Does the added feature your \"shortstatus --mini\"\noffers over \"diff-index --cached --exit-code\" justify the latency penalty\nto the user?\n"},{"id":"103946","messageId":"slrngp1u4h.i22.sitaramc@sitaramc.homelinux.net","threadId":"17688","inReplyTo":"7vr627qd4p.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2009-02-10T03:46:26Z","receivedAt":"2009-02-10T03:46:26Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On 2009-02-10, Junio C Hamano wrote:\n> Tuncer Ayaz writes:\n>\n>> Adding git 'shortstatus --mini' to PS1 is not noticeable or 1sec\n>> maximum in my tree. As a worst case it takes 10secs in a clone\n>> of WebKit.git.\n>\n> Frankly, I think having to spend one second to add only one or two bits to\n> PS1 is simply spending one second too much.\n\n[snip]\n\n> Do you really need more than \"diff-index --cached --exit-code\" in your\n> $PS1 code, and so why? Does the added feature your \"shortstatus --mini\"\n> offers over \"diff-index --cached --exit-code\" justify the latency penalty\n> to the user?\n\nI wonder if I could ask people opinions on a trick I pulled,\nwhich is basically maintain a state of the value of $SECONDS\neach time the user is shown a bash prompt. If the value is\nthe same as last time (meaning he hit enter twice in a row\nvery quickly), it runs the extra stuff.\n\nIt sounds like a dirty trick, but seems to work fine and\ngive you the best of both worlds.\n"},{"id":"103961","messageId":"4ac8254d0902100211p6a52e040je10e11c4f79ea488@mail.gmail.com","threadId":"17688","inReplyTo":"7vr627qd4p.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-02-10T10:11:59Z","receivedAt":"2009-02-10T10:11:59Z","isPatch":true,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"On Tue, Feb 10, 2009 at 2:44 AM, Junio C Hamano wrote:\n> Tuncer Ayaz writes:\n>\n>> Adding git 'shortstatus --mini' to PS1 is not noticeable or 1sec\n>> maximum in my tree. As a worst case it takes 10secs in a clone\n>> of WebKit.git.\n\nJunio, if I leave out my --mini experiment would you be interested\nin merging shortstatus without any additions except maybe\nremoving the index_score? Is it useful enough in your eyes? If yes\nI will resubmit it and decouple the --mini case completely as\npossible future work.\n\n> Frankly, I think having to spend one second to add only one or two bits to\n> PS1 is simply spending one second too much.\n\nACK. it will get worse with time.\n\n>> diff --git a/builtin-commit.c b/builtin-commit.c\n>> index d6a3a62..9267d26 100644\n>> --- a/builtin-commit.c\n>> +++ b/builtin-commit.c\n>> @@ -821,6 +827,88 @@ static int parse_and_validate_options(int argc, const char *argv[],\n>> return argc;\n>> }\n>>\n>> +int cmd_shortstatus(int argc, const char **argv, const char *prefix)\n>> +{\n>> + struct wt_status s;\n>> + int i;\n>> + int c, a, u;\n>> +\n>> + c = a = u = 0;\n>> +\n>> + argc = parse_and_validate_options(argc, argv, builtin_shortstatus_usage, prefix);\n>> + read_cache();\n>> + refresh_cache(REFRESH_QUIET);\n>> + wt_status_prepare(&s);\n>> + wt_status_collect_changes(&s);\n>> + if (mini) {\n>> + for (i = 0; i < s.change.nr; i++) {\n>> + struct wt_status_change_data *d;\n>> + struct string_list_item *it;\n>> +\n>> + it = &(s.change.items[i]);\n>> + d = it->util;\n>> + switch (d->index_status) {\n>> + case DIFF_STATUS_ADDED:\n>> + a = 1;\n>> + break;\n>> + case 0:\n>> + case DIFF_STATUS_COPIED:\n>> + case DIFF_STATUS_DELETED:\n>> + case DIFF_STATUS_MODIFIED:\n>> + case DIFF_STATUS_RENAMED:\n>> + case DIFF_STATUS_TYPE_CHANGED:\n>> + c = 1;\n>> + break;\n>\n> If you at the end discard information by squashing renamed, copied,\n> deleted and modified into a single \"changed\" category, I do not think you\n> would want wt_status_collect_changes() to spend the cost of rename\n> detection in the first place. Sure, you can tell between \"git mv old new\"\n> and \"git add new\", because you won't show \"+\" for \"new\" if you run rename\n> detection, but that is about the only thing I think you are getting.\n\nactually I can leave out all but case 0 to get the current behavior.\nI am not sure but (presumably) have the suspicion from what I\nhave read that these extra cases are irrelevant in this case.\nI may err.\n\n> Is it worth extra 1 second (or 10 seconds)?\n\n1 second is noticeable and therefore bad but it is a definite\nimprovement compared to what I had before with\n'git status|grep' calls. it is slow for PS1, yes.\n\n> What are you really trying to achieve? Do you want to see if you have any\n> change to the index since you checked out? Do you want to further tell\n> the user that the work tree has more changes that are not staged yet\n> (which --mini does not seem to do)?\n>\n> Do you really need more than \"diff-index --cached --exit-code\" in your\n> $PS1 code, and so why? Does the added feature your \"shortstatus --mini\"\n> offers over \"diff-index --cached --exit-code\" justify the latency penalty\n> to the user?\n>\n\nWhat I and others need - based on the fact that the PS1\nenhancement was inspired by someone else's PS1 - is not\ndiff-index --cached. It should include changes in the\nindex plus those not.\nThe feature is there to display that a repo is dirty and if\npossible in an instant way also display that there are not\nonly untrackeds but also modifications and/or additions not\nyet committed with separate symbols (+,*,?).\nIf this is not currently implementable fast enough let's forget\nabout it for now and tackle it once the future unfolds and shows\nus a better path or someone comes up with a bright idea :).\n"},{"id":"103963","messageId":"alpine.DEB.1.00.0902101120460.10279@pacific.mpi-cbg.de","threadId":"17688","inReplyTo":"slrngp1u4h.i22.sitaramc@sitaramc.homelinux.net","subject":"Spending time in PS1, was Re: [RFC/PATCH] shortstatus v1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-02-10T10:22:46Z","receivedAt":"2009-02-10T10:22:46Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Feb 2009, Sitaram Chamarty wrote:\n\n> I wonder if I could ask people opinions on a trick I pulled, which is \n> basically maintain a state of the value of $SECONDS each time the user \n> is shown a bash prompt. If the value is the same as last time (meaning \n> he hit enter twice in a row very quickly), it runs the extra stuff.\n\nAs you know, I am a big fan of consistency. In this light, I do not like \nit when I am shown something at times, and at other times not.\n\nBesides, I agree with Junio that even a single second spent to calculate \nPS1 is too much. I use my command line extensively (some claim I live in \nit). An slow PS1 would drive me to madness.\n\nCiao,\nDscho\n"},{"id":"103970","messageId":"20090210110330.GB12089@coredump.intra.peff.net","threadId":"17688","inReplyTo":"1234227067-56666-1-git-send-email-tuncer.ayaz@gmail.com","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T11:03:30Z","receivedAt":"2009-02-10T11:03:30Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 01:51:07AM +0100, Tuncer Ayaz wrote:\n\n> As discussed recently I started taking Junio's shortstatus patch\n> from October 25th 2008 and integrated it into current master.\n> \n> This revision does work as advertised by Junio and v0 also did.\n\nI did a simple test with this:\n\n mkdir repo && cd repo && git init &&\n touch unchanged changed changed-staged deleted deleted-staged &&\n git add . && git commit -m one &&\n echo changes >changed &&\n echo changes >changed-staged && git add changed-staged &&\n rm deleted &&\n git rm deleted-staged &&\n git shortstatus\n\nThe output is:\n\n changed\n M changed-staged\n deleted\n D deleted-staged\n\nSome comments:\n\n 1. Is the staggered indentation intentional? It looks awful, and the\n only use I can think of is to separate unstaged from staged\n changes. But surely there must be a more obvious way of doing so.\n\n 2. Why do staged changes get a letter marking what happened, but\n unstaged changes do not?\n\n 3. What advantage does this have over just doing:\n\n (git diff --name-status;\n git diff --cached --name-status) | sort -k2\n\n> Right now this is basically Junio's shortstatus\n> from Oct 25th 2008 with no substantial change\n> except a line or two.\n\nThis is not a very helpful commit message. What is it supposed to do?\nWhat does the output look like? Why is it implemented this way? If Junio\nsent a patch in October and it isn't substantially changed, why wasn't\nit accepted then?\n\n> +static const char * const builtin_shortstatus_usage[] = {\n> +\t\"git shortstatus [options] [--] ...\",\n> +\tNULL\n> +};\n\nReally? Doing \"git shortstatus subdir\" seems not to affect the output,\nnor does \"git shortstatus I totally made up these command line\narguments\".\n\nWhat options are available? It looks like this is intimately tied with\n\"commit\", which I think is one of the _shortcomings_ of the current\nstatus. It means the command line options are non-intuitive for what\npeople generally want to say: \"what is changed, possibly limiting to\nsome path\".\n\n> +\tOPT_BOOLEAN(0, \"mini\", &mini, \"print mini shortstatus\"),\n\nSo now \"git status --mini\" doesn't complain, but it doesn't seem to\nactually do anything.\n\n> +\targc = parse_and_validate_options(argc, argv, builtin_shortstatus_usage, prefix);\n\nAh, I see the source of the option issues. You parse with the commit\noptions, but then you don't actually respect any of them. You would want\na totally separate set of options for shortstatus. In fact, I really\ndon't see what point there is in putting it with the 'commit' code at\nall.\n\n> +\tif (mini) {\n> +\t\tfor (i = 0; i < s.change.nr; i++) {\n> +\t\t\tstruct wt_status_change_data *d;\n> +\t\t\tstruct string_list_item *it;\n> +\n> +\t\t\tit = &(s.change.items[i]);\n> +\t\t\td = it->util;\n> +\t\t\tswitch (d->index_status) {\n> +\t\t\t\tcase DIFF_STATUS_ADDED:\n> +\t\t\t\t\ta = 1;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tcase 0:\n> +\t\t\t\tcase DIFF_STATUS_COPIED:\n> +\t\t\t\tcase DIFF_STATUS_DELETED:\n> +\t\t\t\tcase DIFF_STATUS_MODIFIED:\n> +\t\t\t\tcase DIFF_STATUS_RENAMED:\n> +\t\t\t\tcase DIFF_STATUS_TYPE_CHANGED:\n> +\t\t\t\t\tc = 1;\n> +\t\t\t\t\tbreak;\n> +\t\t\t\tdefault:\n> +\t\t\t\tcase DIFF_STATUS_UNKNOWN:\n> +\t\t\t\tcase DIFF_STATUS_UNMERGED:\n> +\t\t\t\t\tu = 1;\n> +\t\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t\tif (c)\n> +\t\t\tprintf(\"*\");\n> +\t\tif (a)\n> +\t\t\tprintf(\"+\");\n> +\t\tif (u)\n> +\t\t\tprintf(\"?\");\n\nIsn't this a bit heavy-handed? If you really just want to know \"are\nthere any changes\", can't you run a custom diff with EXIT_CODE and QUIET\nset, which will bail when it sees the first change, saving you a lot of\nuseless computation?\n\n> +\t} else {\n> +\t\tfor (i = 0; i < s.change.nr; i++) {\n> +\t\t\tstruct wt_status_change_data *d;\n> +\t\t\tstruct string_list_item *it;\n> +\t\t\tchar pfx[1 + 3 + 1 + 1];\n\nHoly magic numbers, Batman.\n\n-Peff\n"},{"id":"103977","messageId":"49916524.4000400@drmicha.warpmail.net","threadId":"17688","inReplyTo":"20090210110330.GB12089@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-02-10T11:29:40Z","receivedAt":"2009-02-10T11:29:40Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 10.02.2009 12:03:\n> On Tue, Feb 10, 2009 at 01:51:07AM +0100, Tuncer Ayaz wrote:\n...\n> 3. What advantage does this have over just doing:\n> \n> (git diff --name-status;\n> git diff --cached --name-status) | sort -k2\n\nThat is fine, except that it can't list untracked files.\n\n> What options are available? It looks like this is intimately tied with\n> \"commit\", which I think is one of the _shortcomings_ of the current\n> status. It means the command line options are non-intuitive for what\n> people generally want to say: \"what is changed, possibly limiting to\n> some path\".\n\nRight now, \"git status\" is basically \"git commit --dry-run\", which may\nor may not be good, but certainly is not what people coming from other\nvcs expect. I would suggest having \"git commit -n\" replace \"git status\"\nif I hadn't done so already or if I dared to (I can't remember ;) ).\n\nThe softer approach was naming \"shortstatus\" what those people would\nexpect for \"status\".\n\nThe \"git diff\" based solution does almost everything, but back then it\nwasn't clear how to get at the untracked and ignored files. In fact,\nthat would have the benefit that output from \"git diff --name-status\ncommitA commitB\" is guaranteed to stay consistent with \"git diff\n--name-status HEAD WORKTREE\", \"git diff --name-status INDEX WORKTREE\"\nand the three-way diff between HEAD, INDEX and WORKTREE which\nshortstatus really is (WORKTREE meaning full wt with untrcaked/ignored\nfiles).\n\n\"git ls-files\" may do but has a different set of mode characters. I\nthink that sums up what preceeded Junio's patch from October.\n\nMichael\n"},{"id":"103978","messageId":"4ac8254d0902100331h4f74df6am6cd514e6ba2c8d6a@mail.gmail.com","threadId":"17688","inReplyTo":"49916524.4000400@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-02-10T11:31:57Z","receivedAt":"2009-02-10T11:31:57Z","isPatch":true,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"On Tue, Feb 10, 2009 at 12:29 PM, Michael J Gruber\n wrote:\n> Jeff King venit, vidit, dixit 10.02.2009 12:03:\n>> On Tue, Feb 10, 2009 at 01:51:07AM +0100, Tuncer Ayaz wrote:\n> ...\n>> 3. What advantage does this have over just doing:\n>>\n>> (git diff --name-status;\n>> git diff --cached --name-status) | sort -k2\n>\n> That is fine, except that it can't list untracked files.\n>\n>> What options are available? It looks like this is intimately tied with\n>> \"commit\", which I think is one of the _shortcomings_ of the current\n>> status. It means the command line options are non-intuitive for what\n>> people generally want to say: \"what is changed, possibly limiting to\n>> some path\".\n>\n> Right now, \"git status\" is basically \"git commit --dry-run\", which may\n> or may not be good, but certainly is not what people coming from other\n> vcs expect. I would suggest having \"git commit -n\" replace \"git status\"\n> if I hadn't done so already or if I dared to (I can't remember ;) ).\n>\n> The softer approach was naming \"shortstatus\" what those people would\n> expect for \"status\".\n>\n> The \"git diff\" based solution does almost everything, but back then it\n> wasn't clear how to get at the untracked and ignored files. In fact,\n> that would have the benefit that output from \"git diff --name-status\n> commitA commitB\" is guaranteed to stay consistent with \"git diff\n> --name-status HEAD WORKTREE\", \"git diff --name-status INDEX WORKTREE\"\n> and the three-way diff between HEAD, INDEX and WORKTREE which\n> shortstatus really is (WORKTREE meaning full wt with untrcaked/ignored\n> files).\n>\n> \"git ls-files\" may do but has a different set of mode characters. I\n> think that sums up what preceeded Junio's patch from October.\n\nFor reference:\nhttp://markmail.org/message/tqvshvcj2ybgj6ea\n"},{"id":"103981","messageId":"20090210114506.GF12089@coredump.intra.peff.net","threadId":"17688","inReplyTo":"49916524.4000400@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T11:45:06Z","receivedAt":"2009-02-10T11:45:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 12:29:40PM +0100, Michael J Gruber wrote:\n\n> > 3. What advantage does this have over just doing:\n> > \n> > (git diff --name-status;\n> > git diff --cached --name-status) | sort -k2\n> \n> That is fine, except that it can't list untracked files.\n\nWell, neither does this patch:\n\n $ echo content >tracked &&\n > echo content >untracked &&\n > git add tracked &&\n > git shortstatus\n A tracked\n\nbut you could easily include that:\n\n (git diff --name-status;\n git diff --cached --name-status;\n git ls-files --exclude-standard -o | sed 's/^/? /') | sort -k2\n\nwhich is really more or less what the wt-status code does.\n\nNote that I am not _against_ a convenient command for doing this. But I\nhave to wonder why such a large patch is necessary when I can do it in\nthree lines. I don't mind the C version being a little longer, but I\nwonder what advantage there is in using wt_status for this.\n\n> Right now, \"git status\" is basically \"git commit --dry-run\", which may\n> or may not be good, but certainly is not what people coming from other\n> vcs expect. I would suggest having \"git commit -n\" replace \"git status\"\n> if I hadn't done so already or if I dared to (I can't remember ;) ).\n\nI would much prefer that, if it had been done that way from the\nbeginning. But I think we are stuck with \"git status\" due to hysterical\nraisins.\n\n> \"git ls-files\" may do but has a different set of mode characters. I\n> think that sums up what preceeded Junio's patch from October.\n\nBut you only need to use it here to get the untracked files, so it\ndoesn't matter what it says about modified files.\n\nThe big downside with the snippet I posted above is that it runs three\nseparate commands that go through the index. In theory, you could do it\nin one pass. But wt-status _doesn't_ do that, since the diff\ninfrastructure isn't there (a long time ago, Junio had an experimental\nparallel diff walker patch, but it never made it out of next).\n\n-Peff\n"},{"id":"103992","messageId":"499174E8.3030207@drmicha.warpmail.net","threadId":"17688","inReplyTo":"20090210114506.GF12089@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2009-02-10T12:36:56Z","receivedAt":"2009-02-10T12:36:56Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Jeff King venit, vidit, dixit 10.02.2009 12:45:\n...\n>> Right now, \"git status\" is basically \"git commit --dry-run\", which may\n>> or may not be good, but certainly is not what people coming from other\n>> vcs expect. I would suggest having \"git commit -n\" replace \"git status\"\n>> if I hadn't done so already or if I dared to (I can't remember ;) ).\n> \n> I would much prefer that, if it had been done that way from the\n> beginning. But I think we are stuck with \"git status\" due to hysterical\n> raisins.\n\nROTFTCOOTF!\n\nNow I know why I never liked those caricatures of grapes...\n\n>> \"git ls-files\" may do but has a different set of mode characters. I\n>> think that sums up what preceeded Junio's patch from October.\n> \n> But you only need to use it here to get the untracked files, so it\n> doesn't matter what it says about modified files.\n> \n> The big downside with the snippet I posted above is that it runs three\n> separate commands that go through the index. In theory, you could do it\n> in one pass. But wt-status _doesn't_ do that, since the diff\n> infrastructure isn't there (a long time ago, Junio had an experimental\n> parallel diff walker patch, but it never made it out of next).\n\nWe completely agree. How do you suggest to progress? Go for the diff\nwalker? For a (porc.) command like shortstatus I think going through the\nindex 3 times isn't that bad, all disk access should be cached after the\nfirst run.\n\nMichael\n"},{"id":"103999","messageId":"20090210130153.GA17305@coredump.intra.peff.net","threadId":"17688","inReplyTo":"499174E8.3030207@drmicha.warpmail.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T13:01:53Z","receivedAt":"2009-02-10T13:01:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 01:36:56PM +0100, Michael J Gruber wrote:\n\n> > I would much prefer that, if it had been done that way from the\n> > beginning. But I think we are stuck with \"git status\" due to hysterical\n> > raisins.\n> \n> ROTFTCOOTF!\n\nI have to admit, this is a new acronym for me...I get the first half,\nbut not the second.\n\n> > The big downside with the snippet I posted above is that it runs three\n> > separate commands that go through the index. In theory, you could do it\n> > in one pass. But wt-status _doesn't_ do that, since the diff\n> > infrastructure isn't there (a long time ago, Junio had an experimental\n> > parallel diff walker patch, but it never made it out of next).\n> \n> We completely agree. How do you suggest to progress? Go for the diff\n> walker? For a (porc.) command like shortstatus I think going through the\n> index 3 times isn't that bad, all disk access should be cached after the\n> first run.\n\nI don't know if resurrecting the parallel diff walker is worth the\ntrouble. I guess if somebody cares enough about the performance they can\nfind the old patch and try benchmarking it.\n\nMaking a C command rather than a shell script is probably reasonable if\nthis is performance critical (and there seems to be talk of putting it\ninto a prompt). What I really object to in the patch is:\n\n - sticking this in builtin-commit.c. It really has _nothing_ to do\n with commit or the existing status. Especially using the same\n option parser is just nonsensical.\n\n - I'm not sure bolting this onto wt-status really makes much sense.\n Especially the performance-critical --mini prompt mode _doesn't_\n want to do the string collection because it wastes a lot of cycles\n figuring out things that we are just going to throw away.\n\n However, for the \"regular\" mode, I don't think it is too big a\n problem. wt_status does a few extra things that shortstatus won't\n care about, but I don't think they are too performance critical\n (e.g., I believe it will find out the \"your branch is N commits\n ahead of the remote\" information).\n\n-Peff\n"},{"id":"104028","messageId":"7vwsbynv0o.fsf@gitster.siamese.dyndns.org","threadId":"17688","inReplyTo":"20090210110330.GB12089@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T15:58:47Z","receivedAt":"2009-02-10T15:58:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Some comments:\n>\n> 1. Is the staggered indentation intentional? It looks awful, and the\n> only use I can think of is to separate unstaged from staged\n> changes. But surely there must be a more obvious way of doing so.\n\nProbably not.\n\n> 2. Why do staged changes get a letter marking what happened, but\n> unstaged changes do not?\n\nBug? FWIW, the original patch from October shows:\n\n M changed\nM M changed-again\nM changed-staged\n D deleted\nD deleted-staged\n\n(where changed-again has both staged changes and further changes in the\nwork tree).\n\nThe gap between these two are to show the rename similarity index, which\nwe could do without.\n\n> 3. What advantage does this have over just doing:\n>\n> (git diff --name-status;\n> git diff --cached --name-status) | sort -k2\n>\n>> Right now this is basically Junio's shortstatus\n>> from Oct 25th 2008 with no substantial change\n>> except a line or two.\n>\n> This is not a very helpful commit message. What is it supposed to do?\n> What does the output look like? Why is it implemented this way? If Junio\n> sent a patch in October and it isn't substantially changed, why wasn't\n> it accepted then?\n\nThe output mimicked what was in Shawn's \"repo\" tool announcement IIRC.\n\nMy patch was supposed to give interested parties hint to base a patch like\nTuncer's on (I think this answers your last question, too).\n"},{"id":"104050","messageId":"slrngp3ef2.a51.sitaramc@sitaramc.homelinux.net","threadId":"17688","inReplyTo":"alpine.DEB.1.00.0902101120460.10279@pacific.mpi-cbg.de","subject":"Re: Spending time in PS1, was Re: [RFC/PATCH] shortstatus v1","fromName":"Sitaram Chamarty","fromEmail":"sitaramc@gmail.com","sentAt":"2009-02-10T17:31:14Z","receivedAt":"2009-02-10T17:31:14Z","isPatch":true,"sender":{"key":"sitaramc@gmail.com","avatar":"https://avatars.githubusercontent.com/u/43316?v=4"},"body":"On 2009-02-10, Johannes Schindelin wrote:\n> Hi,\n>\n> On Tue, 10 Feb 2009, Sitaram Chamarty wrote:\n>\n>> I wonder if I could ask people opinions on a trick I pulled, which is \n>> basically maintain a state of the value of $SECONDS each time the user \n>> is shown a bash prompt. If the value is the same as last time (meaning \n>> he hit enter twice in a row very quickly), it runs the extra stuff.\n>\n> As you know, I am a big fan of consistency. In this light, I do not like \n\nActually no; I haven't been here long enough yet.\n\n> it when I am shown something at times, and at other times not.\n\nHowever, it seems to me that this discussion is about\nreconciling two conflicting needs:\n\n - some people (not all) want certain info in the prompt\n - but getting that info is expensive so it shouldn't be in\n the prompt *all* the time\n\nSuch a conflict might well be served by showing something\nsometimes, and at other times not. A little bit of\nDWIMmery, if you will...\n\nIn any case, one can always alias something to a single\nletter (say \"s\") if one needs quick but not \"in your face\"\naccess to this info, making this whole discussion moot if it\nshould not be *in* the prompt.\n"},{"id":"104054","messageId":"20090210181052.GA19634@coredump.intra.peff.net","threadId":"17688","inReplyTo":"7vwsbynv0o.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T18:10:52Z","receivedAt":"2009-02-10T18:10:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 07:58:47AM -0800, Junio C Hamano wrote:\n\n> > 2. Why do staged changes get a letter marking what happened, but\n> > unstaged changes do not?\n> \n> Bug? FWIW, the original patch from October shows:\n> \n> M changed\n> M M changed-again\n> M changed-staged\n> D deleted\n> D deleted-staged\n> \n> (where changed-again has both staged changes and further changes in the\n> work tree).\n> \n> The gap between these two are to show the rename similarity index, which\n> we could do without.\n\nOK, that makes more sense. And your example shows a good answer to my\nearlier question: why is just sorting the output of the the three\ncommands (diff, diff --cached, and ls-files -o) not as nice. The answer\nis that we need to actually combine lines when files are appear in\nmultiple places.\n\n> The output mimicked what was in Shawn's \"repo\" tool announcement IIRC.\n> \n> My patch was supposed to give interested parties hint to base a patch like\n> Tuncer's on (I think this answers your last question, too).\n\nI went back and read some of the background. I think having this work\nwith the wt-status machinery is reasonable, then. My concerns with\nTuncer's patch are still:\n\n - this should not be part of builtin-commit.c; it doesn't use any of\n the same code except that which has already been lib-ified in\n wt-status.[ch].\n\n - I don't think the \"mini\" status is really related to this. The novel\n thing here is collating the outputs into a single sorted list. But\n the \"mini\" output is not about that at all:\n\n 1. It doesn't care about full output, so it should be able to exit\n early from the diff, avoid rename detection, etc, so that it is\n as quick as possible.\n\n 2. It doesn't collate the output at all. It is about three\n separate symbols for the three separate lists.\n\n-Peff\n\n\n\n\n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n"},{"id":"104055","messageId":"20090210182214.GA19957@coredump.intra.peff.net","threadId":"17688","inReplyTo":"20090210181052.GA19634@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T18:22:14Z","receivedAt":"2009-02-10T18:22:14Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 01:10:52PM -0500, Jeff King wrote:\n\n> - I don't think the \"mini\" status is really related to this. The novel\n> thing here is collating the outputs into a single sorted list. But\n> the \"mini\" output is not about that at all:\n> \n> 1. It doesn't care about full output, so it should be able to exit\n> early from the diff, avoid rename detection, etc, so that it is\n> as quick as possible.\n> \n> 2. It doesn't collate the output at all. It is about three\n> separate symbols for the three separate lists.\n\nOh, sorry, I was misreading the \"mini\" output. I thought the three flags\ncorresponded to the staged, unstaged, and untracked changes. But they\nare \"unstaged or staged but added\", \"unstaged or staged but changed\", or\n\"untracked\" (although right now the last is triggered by unmerged\nentries?).\n\nI honestly don't see much point in differentiating added versus changed\nfiles. Splitting it into \"some things are staged\" and \"some things are\nnot staged\" makes more sense to me. But if you do want that distinction\nthen an early exit from the diff is more complicated (since you might\nhave to keep going to see if there are any of _either_ type).\n\n-Peff\n"},{"id":"104062","messageId":"20090210191118.GA26651@coredump.intra.peff.net","threadId":"17688","inReplyTo":"20090210181052.GA19634@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T19:11:18Z","receivedAt":"2009-02-10T19:11:18Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 01:10:52PM -0500, Jeff King wrote:\n\n> - I don't think the \"mini\" status is really related to this. The novel\n> thing here is collating the outputs into a single sorted list. But\n> the \"mini\" output is not about that at all:\n> \n> 1. It doesn't care about full output, so it should be able to exit\n> early from the diff, avoid rename detection, etc, so that it is\n> as quick as possible.\n> \n> 2. It doesn't collate the output at all. It is about three\n> separate symbols for the three separate lists.\n\nOK, I realize this is not exactly what the proposed --mini does. But\nhere is more along the lines of what I was thinking.\n\nWarm cache, it runs in .042s on my git repo, about half of which is the\nuntracked files check. It takes about .49s on the kernel repo. The\nread_directory() bit is not optimized at all, and could probably benefit\nfrom an early return (OTOH, the worst case is still going to need to\nlook at every path).\n\nI am not particularly interested in a fancy prompt myself, but maybe\nthis will help somebody else.\n\nThe patch relies on the index_differs_from() patch that Stephan\nposted earlier today.\n\n---\n .gitignore | 1 +\n Makefile | 1 +\n builtin-ministatus.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++\n builtin.h | 1 +\n git.c | 1 +\n 5 files changed, 56 insertions(+), 0 deletions(-)\n\ndiff --git a/.gitignore b/.gitignore\nindex 055eb54..de2249b 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -81,6 +81,7 @@ git-mergetool\n git-mktag\n git-mktree\n git-name-rev\n+git-ministatus\n git-mv\n git-notes\n git-pack-redundant\ndiff --git a/Makefile b/Makefile\nindex a0ca137..9145c7b 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -559,6 +559,7 @@ BUILTIN_OBJS += builtin-merge-base.o\n BUILTIN_OBJS += builtin-merge-file.o\n BUILTIN_OBJS += builtin-merge-ours.o\n BUILTIN_OBJS += builtin-merge-recursive.o\n+BUILTIN_OBJS += builtin-ministatus.o\n BUILTIN_OBJS += builtin-mv.o\n BUILTIN_OBJS += builtin-name-rev.o\n BUILTIN_OBJS += builtin-pack-objects.o\ndiff --git a/builtin-ministatus.c b/builtin-ministatus.c\nnew file mode 100644\nindex 0000000..c9f8e7f\n--- /dev/null\n+++ b/builtin-ministatus.c\n@@ -0,0 +1,52 @@\n+#include \"cache.h\"\n+#include \"diff.h\"\n+#include \"commit.h\"\n+#include \"revision.h\"\n+#include \"dir.h\"\n+\n+static int worktree_is_dirty(void)\n+{\n+\tstruct rev_info rev;\n+\tinit_revisions(&rev, \"\");\n+\tsetup_revisions(0, NULL, &rev, NULL);\n+\tDIFF_OPT_SET(&rev.diffopt, QUIET);\n+\tDIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n+\trun_diff_files(&rev, 0);\n+\treturn DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n+}\n+\n+static int have_untracked(void)\n+{\n+\tstruct dir_struct dir;\n+\tint i;\n+\n+\tmemset(&dir, 0, sizeof dir);\n+\tsetup_standard_excludes(&dir);\n+\n+\tread_directory(&dir, \".\", \"\", 0, NULL);\n+\t/* XXX we are probably leaking memory from dir */\n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tstruct dir_entry *ent = dir.entries[i];\n+\t\tif (cache_name_is_other(ent->name, ent->len))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+int cmd_ministatus(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc != 1)\n+\t\tdie(\"Sorry, I don't understand any command line options.\");\n+\n+\tread_cache();\n+\trefresh_cache(REFRESH_QUIET);\n+\n+\tif (index_differs_from(\"HEAD\", 0))\n+\t\tputchar('+');\n+\tif (worktree_is_dirty())\n+\t\tputchar('*');\n+\tif (have_untracked())\n+\t\tputchar('?');\n+\n+\treturn 0;\n+}\ndiff --git a/builtin.h b/builtin.h\nindex f054fc7..03e6a88 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -71,6 +71,7 @@ extern int cmd_merge_base(int argc, const char **argv, const char *prefix);\n extern int cmd_merge_ours(int argc, const char **argv, const char *prefix);\n extern int cmd_merge_file(int argc, const char **argv, const char *prefix);\n extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\n+extern int cmd_ministatus(int argc, const char **argv, const char *prefix);\n extern int cmd_mv(int argc, const char **argv, const char *prefix);\n extern int cmd_name_rev(int argc, const char **argv, const char *prefix);\n extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);\ndiff --git a/git.c b/git.c\nindex 4c0fa44..8bf7e78 100644\n--- a/git.c\n+++ b/git.c\n@@ -323,6 +323,7 @@ static void handle_internal_command(int argc, const char **argv)\n \t\t{ \"merge-ours\", cmd_merge_ours, RUN_SETUP },\n \t\t{ \"merge-recursive\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n+\t\t{ \"ministatus\", cmd_ministatus, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"mv\", cmd_mv, RUN_SETUP | NEED_WORK_TREE },\n \t\t{ \"name-rev\", cmd_name_rev, RUN_SETUP },\n \t\t{ \"pack-objects\", cmd_pack_objects, RUN_SETUP },\n"},{"id":"104081","messageId":"4ac8254d0902101321w1b6171cfkf7a6253181324acd@mail.gmail.com","threadId":"17688","inReplyTo":"20090210191118.GA26651@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-02-10T21:21:16Z","receivedAt":"2009-02-10T21:21:16Z","isPatch":true,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"On Tue, Feb 10, 2009 at 8:11 PM, Jeff King wrote:\n> On Tue, Feb 10, 2009 at 01:10:52PM -0500, Jeff King wrote:\n>\n>> - I don't think the \"mini\" status is really related to this. The novel\n>> thing here is collating the outputs into a single sorted list. But\n>> the \"mini\" output is not about that at all:\n>>\n>> 1. It doesn't care about full output, so it should be able to exit\n>> early from the diff, avoid rename detection, etc, so that it is\n>> as quick as possible.\n>>\n>> 2. It doesn't collate the output at all. It is about three\n>> separate symbols for the three separate lists.\n>\n> OK, I realize this is not exactly what the proposed --mini does. But\n> here is more along the lines of what I was thinking.\n>\n> Warm cache, it runs in .042s on my git repo, about half of which is the\n> untracked files check. It takes about .49s on the kernel repo. The\n> read_directory() bit is not optimized at all, and could probably benefit\n> from an early return (OTOH, the worst case is still going to need to\n> look at every path).\n>\n> I am not particularly interested in a fancy prompt myself, but maybe\n> this will help somebody else.\n\nI tried this and it did not run faster than my experiment.\nI had to add a missing opening curly brace in\nhave_untracked() before it compiled.\n\nAs we haven't found a fast way yet I can live without it.\n\n> The patch relies on the index_differs_from() patch that Stephan\n> posted earlier today.\n>\n> ---\n> .gitignore | 1 +\n> Makefile | 1 +\n> builtin-ministatus.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> builtin.h | 1 +\n> git.c | 1 +\n> 5 files changed, 56 insertions(+), 0 deletions(-)\n>\n> diff --git a/.gitignore b/.gitignore\n> index 055eb54..de2249b 100644\n> --- a/.gitignore\n> +++ b/.gitignore\n> @@ -81,6 +81,7 @@ git-mergetool\n> git-mktag\n> git-mktree\n> git-name-rev\n> +git-ministatus\n> git-mv\n> git-notes\n> git-pack-redundant\n> diff --git a/Makefile b/Makefile\n> index a0ca137..9145c7b 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -559,6 +559,7 @@ BUILTIN_OBJS += builtin-merge-base.o\n> BUILTIN_OBJS += builtin-merge-file.o\n> BUILTIN_OBJS += builtin-merge-ours.o\n> BUILTIN_OBJS += builtin-merge-recursive.o\n> +BUILTIN_OBJS += builtin-ministatus.o\n> BUILTIN_OBJS += builtin-mv.o\n> BUILTIN_OBJS += builtin-name-rev.o\n> BUILTIN_OBJS += builtin-pack-objects.o\n> diff --git a/builtin-ministatus.c b/builtin-ministatus.c\n> new file mode 100644\n> index 0000000..c9f8e7f\n> --- /dev/null\n> +++ b/builtin-ministatus.c\n> @@ -0,0 +1,52 @@\n> +#include \"cache.h\"\n> +#include \"diff.h\"\n> +#include \"commit.h\"\n> +#include \"revision.h\"\n> +#include \"dir.h\"\n> +\n> +static int worktree_is_dirty(void)\n> +{\n> + struct rev_info rev;\n> + init_revisions(&rev, \"\");\n> + setup_revisions(0, NULL, &rev, NULL);\n> + DIFF_OPT_SET(&rev.diffopt, QUIET);\n> + DIFF_OPT_SET(&rev.diffopt, EXIT_WITH_STATUS);\n> + run_diff_files(&rev, 0);\n> + return DIFF_OPT_TST(&rev.diffopt, HAS_CHANGES);\n> +}\n> +\n> +static int have_untracked(void)\n> +{\n> + struct dir_struct dir;\n> + int i;\n> +\n> + memset(&dir, 0, sizeof dir);\n> + setup_standard_excludes(&dir);\n> +\n> + read_directory(&dir, \".\", \"\", 0, NULL);\n> + /* XXX we are probably leaking memory from dir */\n> + for (i = 0; i < dir.nr; i++)\n> + struct dir_entry *ent = dir.entries[i];\n> + if (cache_name_is_other(ent->name, ent->len))\n> + return 1;\n> + }\n> + return 0;\n> +}\n> +\n> +int cmd_ministatus(int argc, const char **argv, const char *prefix)\n> +{\n> + if (argc != 1)\n> + die(\"Sorry, I don't understand any command line options.\");\n> +\n> + read_cache();\n> + refresh_cache(REFRESH_QUIET);\n> +\n> + if (index_differs_from(\"HEAD\", 0))\n> + putchar('+');\n> + if (worktree_is_dirty())\n> + putchar('*');\n> + if (have_untracked())\n> + putchar('?');\n> +\n> + return 0;\n> +}\n> diff --git a/builtin.h b/builtin.h\n> index f054fc7..03e6a88 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -71,6 +71,7 @@ extern int cmd_merge_base(int argc, const char **argv, const char *prefix);\n> extern int cmd_merge_ours(int argc, const char **argv, const char *prefix);\n> extern int cmd_merge_file(int argc, const char **argv, const char *prefix);\n> extern int cmd_merge_recursive(int argc, const char **argv, const char *prefix);\n> +extern int cmd_ministatus(int argc, const char **argv, const char *prefix);\n> extern int cmd_mv(int argc, const char **argv, const char *prefix);\n> extern int cmd_name_rev(int argc, const char **argv, const char *prefix);\n> extern int cmd_pack_objects(int argc, const char **argv, const char *prefix);\n> diff --git a/git.c b/git.c\n> index 4c0fa44..8bf7e78 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -323,6 +323,7 @@ static void handle_internal_command(int argc, const char **argv)\n> { \"merge-ours\", cmd_merge_ours, RUN_SETUP },\n> { \"merge-recursive\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n> { \"merge-subtree\", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },\n> + { \"ministatus\", cmd_ministatus, RUN_SETUP | NEED_WORK_TREE },\n> { \"mv\", cmd_mv, RUN_SETUP | NEED_WORK_TREE },\n> { \"name-rev\", cmd_name_rev, RUN_SETUP },\n> { \"pack-objects\", cmd_pack_objects, RUN_SETUP },\n>\n"},{"id":"104087","messageId":"20090210213634.GA26954@coredump.intra.peff.net","threadId":"17688","inReplyTo":"4ac8254d0902101321w1b6171cfkf7a6253181324acd@mail.gmail.com","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T21:36:34Z","receivedAt":"2009-02-10T21:36:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 10:21:16PM +0100, Tuncer Ayaz wrote:\n\n> I tried this and it did not run faster than my experiment.\n\nI'm surprised, based on the numbers you gave before. I guess your\nmachine is just a lot slower than what I am experimenting with. I think\nyou will have to profile to see what is taking so long, then, if you\nwant to speed it up.\n\n> I had to add a missing opening curly brace in\n> have_untracked() before it compiled.\n\nSorry about that. I added the comment just above directly into the\npatch, and obviously managed to butcher the brace while I was doing it.\n\n-Peff\n"},{"id":"104091","messageId":"7vtz72kjz0.fsf@gitster.siamese.dyndns.org","threadId":"17688","inReplyTo":"20090210191118.GA26651@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T22:25:39Z","receivedAt":"2009-02-10T22:25:39Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> Warm cache, it runs in .042s on my git repo, about half of which is the\n> untracked files check. It takes about .49s on the kernel repo. The\n> read_directory() bit is not optimized at all, and could probably benefit\n> from an early return (OTOH, the worst case is still going to need to\n> look at every path).\n\nI suspect that with a large tree your have_untracked() would show\nunnecessary overhead from dir_add_name(), because you only want one bit of\ninformation but there is no way to stop with \"ok, we know enough\". This\ntoy patch adds a trivial \"early return\" to read_directory() codepath, but\nthere are two sad things about it.\n\n * In order to cheaply run \"is there a single other file\", you really\n should scan the level you have already opened first before digging\n deeper. I didn't bother because the primary use of read_directory is\n the depth first traversal.\n\n * In a cloned work tree, the tracked files and directories come early in\n the physical directory and then crufts you created yourself comes at\n the end in readdir() order. We tend to read a lot of tracked ones\n first and the finally hit other files.\n\n builtin-ministatus.c | 4 +++-\n dir.c | 32 +++++++++++++++++++++++++++-----\n dir.h | 2 ++\n 3 files changed, 32 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin-ministatus.c b/builtin-ministatus.c\nindex aff9e5a..4b5a191 100644\n--- a/builtin-ministatus.c\n+++ b/builtin-ministatus.c\n@@ -25,7 +25,7 @@ static int have_untracked(void)\n \n \tread_directory(&dir, \".\", \"\", 0, NULL);\n \t/* XXX we are probably leaking memory from dir */\n-\tfor (i = 0; i < dir.nr; i++)\n+\tfor (i = 0; i < dir.nr; i++) {\n \t\tstruct dir_entry *ent = dir.entries[i];\n \t\tif (cache_name_is_other(ent->name, ent->len))\n \t\t\treturn 1;\n@@ -47,6 +47,8 @@ int cmd_ministatus(int argc, const char **argv, const char *prefix)\n \t\tputchar('*');\n \tif (have_untracked())\n \t\tputchar('?');\n+\tif (untracked_files_exist())\n+\t\tputchar('%');\n \n \treturn 0;\n }\ndiff --git a/dir.c b/dir.c\nindex cfd1ea5..8d4fcdd 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -16,7 +16,10 @@ struct path_simplify {\n \n static int read_directory_recursive(struct dir_struct *dir,\n \tconst char *path, const char *base, int baselen,\n-\tint check_only, const struct path_simplify *simplify);\n+\tint mode, const struct path_simplify *simplify);\n+#define READ_DIRECTORY_EMPTY_CHECK 1\n+#define READ_DIRECTORY_OTHER_CHECK 2\n+\n static int get_dtype(struct dirent *de, const char *path);\n \n int common_prefix(const char **pathspec)\n@@ -505,7 +508,8 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n \t/* This is the \"show_other_directories\" case */\n \tif (!dir->hide_empty_directories)\n \t\treturn show_directory;\n-\tif (!read_directory_recursive(dir, dirname, dirname, len, 1, simplify))\n+\tif (!read_directory_recursive(dir, dirname, dirname, len,\n+\t\t\t\t READ_DIRECTORY_EMPTY_CHECK, simplify))\n \t\treturn ignore_directory;\n \treturn show_directory;\n }\n@@ -574,10 +578,12 @@ static int get_dtype(struct dirent *de, const char *path)\n * Also, we ignore the name \".git\" (even if it is not a directory).\n * That likely will not change.\n */\n-static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only, const struct path_simplify *simplify)\n+static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int mode, const struct path_simplify *simplify)\n {\n \tDIR *fdir = opendir(path);\n \tint contents = 0;\n+\tint empty_check_only = (mode == READ_DIRECTORY_EMPTY_CHECK);\n+\tint other_check_only = (mode == READ_DIRECTORY_OTHER_CHECK);\n \n \tif (fdir) {\n \t\tstruct dirent *de;\n@@ -639,7 +645,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\t\t\tbreak;\n \t\t\t\tcase recurse_into_directory:\n \t\t\t\t\tcontents += read_directory_recursive(dir,\n-\t\t\t\t\t\tfullname, fullname, baselen + len, 0, simplify);\n+\t\t\t\t\t\tfullname, fullname, baselen + len, mode, simplify);\n \t\t\t\t\tcontinue;\n \t\t\t\tcase ignore_directory:\n \t\t\t\t\tcontinue;\n@@ -650,10 +656,12 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tcontents++;\n-\t\t\tif (check_only)\n+\t\t\tif (empty_check_only)\n \t\t\t\tgoto exit_early;\n \t\t\telse\n \t\t\t\tdir_add_name(dir, fullname, baselen + len);\n+\t\t\tif (other_check_only && dir->nr)\n+\t\t\t\tgoto exit_early;\n \t\t}\n exit_early:\n \t\tclosedir(fdir);\n@@ -731,6 +739,20 @@ int read_directory(struct dir_struct *dir, const char *path, const char *base, i\n \treturn dir->nr;\n }\n \n+int untracked_files_exist(void)\n+{\n+\tstruct dir_struct dir;\n+\tint i;\n+\n+\tmemset(&dir, 0, sizeof(dir));\n+\tsetup_standard_excludes(&dir);\n+\tread_directory_recursive(&dir, \".\", \"\", 0, READ_DIRECTORY_OTHER_CHECK,\n+\t\t\t\t NULL);\n+\tfor (i = 0; i < dir.nr; i++)\n+\t\tfree(dir.entries[i]);\n+\treturn !!dir.nr;\n+}\n+\n int file_exists(const char *f)\n {\n \tstruct stat sb;\ndiff --git a/dir.h b/dir.h\nindex bdc2d47..1f8b575 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -92,4 +92,6 @@ extern int remove_dir_recursively(struct strbuf *path, int only_empty);\n /* tries to remove the path with empty directories along it, ignores ENOENT */\n extern int remove_path(const char *path);\n \n+extern int untracked_files_exist(void);\n+\n #endif\n"},{"id":"104099","messageId":"4ac8254d0902101452u33a1ef0mb9d34182eff5838f@mail.gmail.com","threadId":"17688","inReplyTo":"7vtz72kjz0.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Tuncer Ayaz","fromEmail":"tuncer.ayaz@gmail.com","sentAt":"2009-02-10T22:52:47Z","receivedAt":"2009-02-10T22:52:47Z","isPatch":true,"sender":{"key":"tuncer.ayaz@gmail.com","avatar":null},"body":"On Tue, Feb 10, 2009 at 11:25 PM, Junio C Hamano wrote:\n> Jeff King writes:\n>\n>> Warm cache, it runs in .042s on my git repo, about half of which is the\n>> untracked files check. It takes about .49s on the kernel repo. The\n>> read_directory() bit is not optimized at all, and could probably benefit\n>> from an early return (OTOH, the worst case is still going to need to\n>> look at every path).\n>\n> I suspect that with a large tree your have_untracked() would show\n> unnecessary overhead from dir_add_name(), because you only want one bit of\n> information but there is no way to stop with \"ok, we know enough\". This\n> toy patch adds a trivial \"early return\" to read_directory() codepath, but\n> there are two sad things about it.\n>\n> * In order to cheaply run \"is there a single other file\", you really\n> should scan the level you have already opened first before digging\n> deeper. I didn't bother because the primary use of read_directory is\n> the depth first traversal.\n>\n> * In a cloned work tree, the tracked files and directories come early in\n> the physical directory and then crufts you created yourself comes at\n> the end in readdir() order. We tend to read a lot of tracked ones\n> first and the finally hit other files.\n\nI've done some measurements from within .bashrc via a\ncustom timer() bash function called before and after git ministatus call.\nThe box I'm testing on is a Core2Duo with 1.8GHz and 2GB of RAM.\nI have faster machines I can test on but do Git coding on this one.\n\nFor WebKit.git this is similar to the previous patch. It does also take\nat least 8 or 9 seconds. That's an improvement over 8-11secs.\n\nIs it good to test against WebKit.git? I mean it's apparently big.\n\n> builtin-ministatus.c | 4 +++-\n> dir.c | 32 +++++++++++++++++++++++++++-----\n> dir.h | 2 ++\n> 3 files changed, 32 insertions(+), 6 deletions(-)\n>\n> diff --git a/builtin-ministatus.c b/builtin-ministatus.c\n> index aff9e5a..4b5a191 100644\n> --- a/builtin-ministatus.c\n> +++ b/builtin-ministatus.c\n> @@ -25,7 +25,7 @@ static int have_untracked(void)\n>\n> read_directory(&dir, \".\", \"\", 0, NULL);\n> /* XXX we are probably leaking memory from dir */\n> - for (i = 0; i < dir.nr; i++)\n> + for (i = 0; i < dir.nr; i++) {\n> struct dir_entry *ent = dir.entries[i];\n> if (cache_name_is_other(ent->name, ent->len))\n> return 1;\n> @@ -47,6 +47,8 @@ int cmd_ministatus(int argc, const char **argv, const char *prefix)\n> putchar('*');\n> if (have_untracked())\n> putchar('?');\n> + if (untracked_files_exist())\n> + putchar('%');\n>\n> return 0;\n> }\n> diff --git a/dir.c b/dir.c\n> index cfd1ea5..8d4fcdd 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -16,7 +16,10 @@ struct path_simplify {\n>\n> static int read_directory_recursive(struct dir_struct *dir,\n> const char *path, const char *base, int baselen,\n> - int check_only, const struct path_simplify *simplify);\n> + int mode, const struct path_simplify *simplify);\n> +#define READ_DIRECTORY_EMPTY_CHECK 1\n> +#define READ_DIRECTORY_OTHER_CHECK 2\n> +\n> static int get_dtype(struct dirent *de, const char *path);\n>\n> int common_prefix(const char **pathspec)\n> @@ -505,7 +508,8 @@ static enum directory_treatment treat_directory(struct dir_struct *dir,\n> /* This is the \"show_other_directories\" case */\n> if (!dir->hide_empty_directories)\n> return show_directory;\n> - if (!read_directory_recursive(dir, dirname, dirname, len, 1, simplify))\n> + if (!read_directory_recursive(dir, dirname, dirname, len,\n> + READ_DIRECTORY_EMPTY_CHECK, simplify))\n> return ignore_directory;\n> return show_directory;\n> }\n> @@ -574,10 +578,12 @@ static int get_dtype(struct dirent *de, const char *path)\n> * Also, we ignore the name \".git\" (even if it is not a directory).\n> * That likely will not change.\n> */\n> -static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int check_only, const struct path_simplify *simplify)\n> +static int read_directory_recursive(struct dir_struct *dir, const char *path, const char *base, int baselen, int mode, const struct path_simplify *simplify)\n> {\n> DIR *fdir = opendir(path);\n> int contents = 0;\n> + int empty_check_only = (mode == READ_DIRECTORY_EMPTY_CHECK);\n> + int other_check_only = (mode == READ_DIRECTORY_OTHER_CHECK);\n>\n> if (fdir) {\n> struct dirent *de;\n> @@ -639,7 +645,7 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n> break;\n> case recurse_into_directory:\n> contents += read_directory_recursive(dir,\n> - fullname, fullname, baselen + len, 0, simplify);\n> + fullname, fullname, baselen + len, mode, simplify);\n> continue;\n> case ignore_directory:\n> continue;\n> @@ -650,10 +656,12 @@ static int read_directory_recursive(struct dir_struct *dir, const char *path, co\n> break;\n> }\n> contents++;\n> - if (check_only)\n> + if (empty_check_only)\n> goto exit_early;\n> else\n> dir_add_name(dir, fullname, baselen + len);\n> + if (other_check_only && dir->nr)\n> + goto exit_early;\n> }\n> exit_early:\n> closedir(fdir);\n> @@ -731,6 +739,20 @@ int read_directory(struct dir_struct *dir, const char *path, const char *base, i\n> return dir->nr;\n> }\n>\n> +int untracked_files_exist(void)\n> +{\n> + struct dir_struct dir;\n> + int i;\n> +\n> + memset(&dir, 0, sizeof(dir));\n> + setup_standard_excludes(&dir);\n> + read_directory_recursive(&dir, \".\", \"\", 0, READ_DIRECTORY_OTHER_CHECK,\n> + NULL);\n> + for (i = 0; i < dir.nr; i++)\n> + free(dir.entries[i]);\n> + return !!dir.nr;\n> +}\n> +\n> int file_exists(const char *f)\n> {\n> struct stat sb;\n> diff --git a/dir.h b/dir.h\n> index bdc2d47..1f8b575 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -92,4 +92,6 @@ extern int remove_dir_recursively(struct strbuf *path, int only_empty);\n> /* tries to remove the path with empty directories along it, ignores ENOENT */\n> extern int remove_path(const char *path);\n>\n> +extern int untracked_files_exist(void);\n> +\n> #endif\n>\n"},{"id":"104100","messageId":"20090210225539.GC26954@coredump.intra.peff.net","threadId":"17688","inReplyTo":"7vtz72kjz0.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-10T22:55:39Z","receivedAt":"2009-02-10T22:55:39Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 02:25:39PM -0800, Junio C Hamano wrote:\n\n> > Warm cache, it runs in .042s on my git repo, about half of which is the\n> > untracked files check. It takes about .49s on the kernel repo. The\n> > read_directory() bit is not optimized at all, and could probably benefit\n> > from an early return (OTOH, the worst case is still going to need to\n> > look at every path).\n> \n> I suspect that with a large tree your have_untracked() would show\n> unnecessary overhead from dir_add_name(), because you only want one bit of\n> information but there is no way to stop with \"ok, we know enough\". This\n> toy patch adds a trivial \"early return\" to read_directory() codepath, but\n> there are two sad things about it.\n\nMy timings are exactly the same (with my have_untracked removed,\nof course). Due most likely to the fact that I keep my repos clean, so\nwe have to look at every directory to realize there are no files.\n\nHere are a few timings:\n\nFor the relative times of each action, I get (in the linux-2.6 tree):\n\n - nothing (just read_cache / refresh_cache)\n real 0m0.178s\n user 0m0.060s\n sys 0m0.116s\n\n - just index_differs_from (minus nothing = 0.107s)\n real 0m0.285s\n user 0m0.140s\n sys 0m0.140s\n\n - just worktree_is_dirty (minus nothing = 0.0s)\n real 0m0.178s\n user 0m0.048s\n sys 0m0.132s\n\n - just untracked_files_exist (minus nothing = 0.184s)\n real 0m0.362s\n user 0m0.188s\n sys 0m0.176s\n\nSo untracked_files is definitely the worst, but I was surprised that\nthe index to HEAD diff takes so long.\n\nFor just index_differs_from, gprof claims we spend most of our time in\nunpack-trees stuff:\n\n 33.33 0.01 0.01 28301 0.00 0.00 unpack_callback\n 33.33 0.02 0.01 28301 0.00 0.00 unpack_nondirectories\n 33.33 0.03 0.01 26627 0.00 0.00 convert_from_disk\n\nFor just untracked_files_exist, gprof claims we spend most of our time\ndealing with the index name hash (and obviously a bit of time in\nexcluded_1):\n\n 27.27 0.03 0.03 1479931 0.00 0.00 icase_hash\n 18.18 0.05 0.02 74142 0.00 0.00 excluded_1\n 9.09 0.06 0.01 120178 0.00 0.00 lookup_hash_entry\n 9.09 0.07 0.01 49855 0.00 0.00 hash_name\n 9.09 0.08 0.01 26627 0.00 0.00 convert_from_disk\n 9.09 0.09 0.01 24739 0.00 0.00 excluded\n 9.09 0.10 0.01 18 0.56 0.88 grow_hash_table\n\nSo maybe there are speedups to be had there. ISTR the icase_hash stuff\ncame about to support case-challenged filesystems. I haven't looked to\nsee if it could be turned into a ALL_OF_MY_FILESYSTEMS_ARE_SANE\ncompile-time option to get some speedup.\n\n-Peff\n"},{"id":"104103","messageId":"7v3aellwoa.fsf@gitster.siamese.dyndns.org","threadId":"17688","inReplyTo":"20090210225539.GC26954@coredump.intra.peff.net","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-10T23:05:57Z","receivedAt":"2009-02-10T23:05:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> So untracked_files is definitely the worst, but I was surprised that\n> the index to HEAD diff takes so long.\n\nThere is an obvious optimization you can do to \"diff-index --cached\" using\ncache-tree. If your index is really clean, computing the tree object the\nindex would represent (without writing the tree object out) and comparing\nit against HEAD^{tree} may be a tad faster.\n"},{"id":"104120","messageId":"20090211085256.6117@nanako3.lavabit.com","threadId":"17688","inReplyTo":"7vwsbynv0o.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-02-10T23:52:56Z","receivedAt":"2009-02-10T23:52:56Z","isPatch":true,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano :\n\n> Jeff King writes:\n>\n>> Some comments:\n>>\n>> 1. Is the staggered indentation intentional? It looks awful, and the\n>> only use I can think of is to separate unstaged from staged\n>> changes. But surely there must be a more obvious way of doing so.\n>\n> Probably not.\n>\n>> 2. Why do staged changes get a letter marking what happened, but\n>> unstaged changes do not?\n>\n> Bug? FWIW, the original patch from October shows:\n>\n> M changed\n> M M changed-again\n> M changed-staged\n> D deleted\n> D deleted-staged\n>\n> (where changed-again has both staged changes and further changes in the\n> work tree).\n>\n> The gap between these two are to show the rename similarity index, which\n> we could do without.\n\nI have a question. Why do you have the gap for the rename similarity between the two but not between the second status and the filename?\n"},{"id":"104300","messageId":"7vab8sd5vp.fsf@gitster.siamese.dyndns.org","threadId":"17688","inReplyTo":"20090211085256.6117@nanako3.lavabit.com","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-02-11T21:24:10Z","receivedAt":"2009-02-11T21:24:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi writes:\n\n> Quoting Junio C Hamano :\n>\n>> Bug? FWIW, the original patch from October shows:\n>>\n>> M changed\n>> M M changed-again\n>> M changed-staged\n>> D deleted\n>> D deleted-staged\n>>\n>> (where changed-again has both staged changes and further changes in the\n>> work tree).\n>>\n>> The gap between these two are to show the rename similarity index, which\n>> we could do without.\n>\n> I have a question. Why do you have the gap for the rename similarity\n> between the two but not between the second status and the filename?\n\nThere can be renames between the HEAD and the index, but by definition\nthere can never be renames between the index and the work tree, because\nwe do not use untracked files in the work tree for comparison, which means\nthere is no \"new\" files when comparing the index and the work tree.\n\nFor this reason, there is need for similarity indices for the second one.\n"},{"id":"104319","messageId":"20090212004921.GD30231@coredump.intra.peff.net","threadId":"17688","inReplyTo":"7v3aellwoa.fsf@gitster.siamese.dyndns.org","subject":"Re: [RFC/PATCH] shortstatus v1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2009-02-12T00:49:21Z","receivedAt":"2009-02-12T00:49:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Feb 10, 2009 at 03:05:57PM -0800, Junio C Hamano wrote:\n\n> There is an obvious optimization you can do to \"diff-index --cached\" using\n> cache-tree. If your index is really clean, computing the tree object the\n> index would represent (without writing the tree object out) and comparing\n> it against HEAD^{tree} may be a tad faster.\n\nClever, but I think you may just be trading one scenario for \"worst\ncase\" versus another (i.e., now when you _do_ have a difference to do an\nearly return, you still have to touch everything in the cache).\n\nJust for fun, I timed a quick and dirty implementation. It looks like\ngenerating the tree actually ends up taking just a little bit longer,\neven with an unchanged index (which should be the case it speeds up).\n\nBut maybe I just did it wrong. My implementation was basically just:\n\n t = cache_tree();\n if (!cache_tree_fully_valid(t))\n cache_tree_update(t, active_cache, active_nr, 0, 0);\n get_sha1(\"HEAD\", sha1);\n return hashcmp(t->sha1, sha1);\n\n-Peff\n"}]}