{"thread":{"id":"52145","subject":"[RFC] xl command for visualizing recent history","startedAt":"2019-10-29T00:30:42Z","lastAt":"2020-02-07T01:39:31Z","messageCount":13,"participants":["Matthew DeVore","Emily Shaffer","Johannes Schindelin","Phillip Wood","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"385028","messageId":"20191029003023.122196-1-matvore@google.com","threadId":"52145","inReplyTo":null,"subject":"[RFC] xl command for visualizing recent history","fromName":"Matthew DeVore","fromEmail":"matvore@google.com","sentAt":"2019-10-29T00:30:23Z","receivedAt":"2019-10-29T00:30:42Z","isPatch":false,"sender":{"key":"matvore@google.com","avatar":"https://avatars.githubusercontent.com/u/946637?v=4"},"body":"From: Matthew DeVore \n\n\"git xl\" shows a graph of recent history, including all existing\nbranches (unless flagged with a config option) and their upstream\ncounterparts. It is named such because it is easy to type and the\nletter \"x\" looks like a small graph.\n\nLike \"git branch\" it supports filtering the branches shown via\npositional arguments.\n\nBesides just showing the graph, it also associates refs with all visible\ncommits with names in the form of \"h/#\" where # is an incrementing\nindex. After showing the graph, these refs can be used to ergonomically\ninvoke some follow-up command like rebase or diff.\n\nThe test cases show non-trivial output which can be used to get an idea\nfor what the command is good for, though it doesn't capture the\ncoloring.\n\nThe primary goals of this command are:\n\n a) deduce what the user wants to see based on what they haven't pushed\n upstream yet\n b) show the active branches spatially rather than as a linear list (as\n in \"git branch\")\n c) allow the user to easily refer to commits that appeared in the\n output\n\nI considered making the h/# tags stable across invocations such that a\nparticular hash will only be tagged with a different number if ~100\nother hashes are tagged since the hash was last tagged. I didn't\nactually implement it this way, instead opting for always re-numbering\nthe hashes on each invocation. This means the hash number is\npredictable based on the position the hash appears in the output, which\nis probably better that encouraging users to memorize hash numbers (or\nuse them in scripts!).\n\nOmissions I might/will fix depending on feedback:\n\n a) rather than show HEAD in the graph, show when\n possible (i.e. \"[]\" rather than \"[HEAD master]\").\n\n b) don't parse output from `git log` but instead do everything\n in-process.\n\n c) documentation\n---\n Makefile | 1 +\n builtin.h | 1 +\n git.c | 1 +\n t/t4400-xl.sh | 270 ++++++++++++++++++++++++++++\n xl.c | 485 ++++++++++++++++++++++++++++++++++++++++++++++++++\n 5 files changed, 758 insertions(+)\n create mode 100755 t/t4400-xl.sh\n create mode 100644 xl.c\n\ndiff --git a/Makefile b/Makefile\nindex 03b800da0c..491661f848 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1022,20 +1022,21 @@ LIB_OBJS += varint.o\n LIB_OBJS += version.o\n LIB_OBJS += versioncmp.o\n LIB_OBJS += walker.o\n LIB_OBJS += wildmatch.o\n LIB_OBJS += worktree.o\n LIB_OBJS += wrapper.o\n LIB_OBJS += write-or-die.o\n LIB_OBJS += ws.o\n LIB_OBJS += wt-status.o\n LIB_OBJS += xdiff-interface.o\n+LIB_OBJS += xl.o\n LIB_OBJS += zlib.o\n \n BUILTIN_OBJS += builtin/add.o\n BUILTIN_OBJS += builtin/am.o\n BUILTIN_OBJS += builtin/annotate.o\n BUILTIN_OBJS += builtin/apply.o\n BUILTIN_OBJS += builtin/archive.o\n BUILTIN_OBJS += builtin/bisect--helper.o\n BUILTIN_OBJS += builtin/blame.o\n BUILTIN_OBJS += builtin/branch.o\ndiff --git a/builtin.h b/builtin.h\nindex 5cf5df69f7..568d09cf7f 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -241,16 +241,17 @@ int cmd_update_server_info(int argc, const char **argv, const char *prefix);\n int cmd_upload_archive(int argc, const char **argv, const char *prefix);\n int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix);\n int cmd_upload_pack(int argc, const char **argv, const char *prefix);\n int cmd_var(int argc, const char **argv, const char *prefix);\n int cmd_verify_commit(int argc, const char **argv, const char *prefix);\n int cmd_verify_tag(int argc, const char **argv, const char *prefix);\n int cmd_version(int argc, const char **argv, const char *prefix);\n int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n int cmd_worktree(int argc, const char **argv, const char *prefix);\n int cmd_write_tree(int argc, const char **argv, const char *prefix);\n+int cmd_xl(int argc, const char **argv, const char *prefix);\n int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n int cmd_show_ref(int argc, const char **argv, const char *prefix);\n int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n int cmd_replace(int argc, const char **argv, const char *prefix);\n \n #endif\ndiff --git a/git.c b/git.c\nindex ce6ab0ece2..4a1da83a7e 100644\n--- a/git.c\n+++ b/git.c\n@@ -594,20 +594,21 @@ static struct cmd_struct commands[] = {\n \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n \t{ \"upload-pack\", cmd_upload_pack },\n \t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n \t{ \"verify-pack\", cmd_verify_pack },\n \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n \t{ \"version\", cmd_version },\n \t{ \"whatchanged\", cmd_whatchanged, RUN_SETUP },\n \t{ \"worktree\", cmd_worktree, RUN_SETUP | NO_PARSEOPT },\n \t{ \"write-tree\", cmd_write_tree, RUN_SETUP },\n+\t{ \"xl\", cmd_xl, RUN_SETUP },\n };\n \n static struct cmd_struct *get_builtin(const char *s)\n {\n \tint i;\n \tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n \t\tstruct cmd_struct *p = commands + i;\n \t\tif (!strcmp(s, p->cmd))\n \t\t\treturn p;\n \t}\ndiff --git a/t/t4400-xl.sh b/t/t4400-xl.sh\nnew file mode 100755\nindex 0000000000..f6e35bd4da\n--- /dev/null\n+++ b/t/t4400-xl.sh\n@@ -0,0 +1,270 @@\n+#!/bin/sh\n+\n+test_description='git xl'\n+. ./test-lib.sh\n+\n+xl () {\n+\tgit xl \"$@\" >actual_raw &&\n+\tsed -e \"s/ *$//\" actual_raw\n+}\n+\n+test_expect_success 'basic' '\n+\ttest_commit foo &&\n+\tgit checkout -b branch2 &&\n+\ttest_commit bar &&\n+\n+\txl >actual &&\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch2]\n+ | bar\n+ |\n+$hashvl2 * 2 committer@example.com [master]\n+ foo\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'specify ref names' '\n+\txl master >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD]\n+ | bar\n+ |\n+$hashvl2 * 2 committer@example.com [master]\n+ foo\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'deduce graph base' '\n+\tgit checkout -b branch3 master &&\n+\ttest_commit baz &&\n+\tgit branch -d master &&\n+\txl >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\txl_base=$(git rev-parse xl_base | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch3]\n+ | baz\n+ |\n+$hashvl2 | * 2 committer@example.com [branch2]\n+ |/ bar\n+ |\n+$xl_base * 3 committer@example.com\n+ foo\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'show upstream branch' '\n+\tgit init --bare upstream_repo.git &&\n+\tgit remote add upstream_repo upstream_repo.git &&\n+\n+\tgit push -u upstream_repo HEAD &&\n+\tgit branch --set-upstream-to=upstream_repo/branch3 &&\n+\ttest_commit not_yet_pushed &&\n+\n+\t# Exclude branch2 by requesting at least one other ref explicitly.\n+\txl branch3 >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch3]\n+ | not_yet_pushed\n+ |\n+$hashvl2 * 2 committer@example.com [upstream_repo/branch3]\n+ baz\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'de-dupe upstream branches' '\n+\tgit checkout -b branch4 upstream_repo/branch3 &&\n+\ttest_commit baz4 &&\n+\n+\t# Make sure we do not show the same upstream branch name twice\n+\t# even though two local branches share the same upstream branch.\n+\txl >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n+\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n+\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch4]\n+ | baz4\n+ |\n+$hashvl2 | * 2 committer@example.com [branch3]\n+ |/ not_yet_pushed\n+ |\n+$hashvl3 * 3 committer@example.com [upstream_repo/branch3]\n+ | baz\n+ |\n+$hashvl4 | * 4 committer@example.com [branch2]\n+ |/ bar\n+ |\n+$hashvl5 * 5 committer@example.com\n+ foo\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'multiple merge bases' '\n+\tgit merge -m merge1 branch3 &&\n+\ttest_commit baz5 &&\n+\n+\tgit checkout branch3 &&\n+\tgit merge -m merge2 h/1 &&\n+\ttest_commit baz6 &&\n+\n+\tgit branch --unset-upstream branch3 &&\n+\txl branch3 branch4 >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n+\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n+\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n+\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch3]\n+ | baz6\n+ |\n+$hashvl2 * 2 committer@example.com\n+ |\\ merge2\n+ | |\n+$hashvl3 | | * 3 committer@example.com [branch4]\n+ | | | baz5\n+ | | |\n+$hashvl4 | | * 4 committer@example.com\n+ | | |\\ merge1\n+ | |/ /\n+ | | /\n+ | |/\n+ |/|\n+$hashvl5 * | 5 committer@example.com\n+ / not_yet_pushed\n+ |\n+$hashvl6 * 6 committer@example.com\n+ baz4\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'orphan branches' '\n+\t# If there are some branches to display which do not have a common\n+\t# ancestor with the other branches, we show them in a separate graph.\n+\tgit checkout --orphan branch-a h/6 &&\n+\tgit commit -m baz7 &&\n+\txl >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n+\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n+\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n+\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n+\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n+\thashvl8=$(git rev-parse h/8 | test_copy_bytes 8) &&\n+\thashvl9=$(git rev-parse h/9 | test_copy_bytes 8) &&\n+\thashv10=$(git rev-parse h/10 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [HEAD branch-a]\n+ baz7\n+\n+$hashvl2 * 2 committer@example.com [branch3]\n+ | baz6\n+ |\n+$hashvl3 * 3 committer@example.com\n+ |\\ merge2\n+ | |\n+$hashvl4 | | * 4 committer@example.com [branch4]\n+ | | | baz5\n+ | | |\n+$hashvl5 | | * 5 committer@example.com\n+ | | |\\ merge1\n+ | |/ /\n+ | | /\n+ | |/\n+ |/|\n+$hashvl6 * | 6 committer@example.com\n+ | | not_yet_pushed\n+ | |\n+$hashvl7 | * 7 committer@example.com\n+ |/ baz4\n+ |\n+$hashvl8 * 8 committer@example.com\n+ | baz\n+ |\n+$hashvl9 | * 9 committer@example.com [branch2]\n+ |/ bar\n+ |\n+$hashv10 * 10 committer@example.com\n+ foo\n+\" >expect &&\n+\ttest_cmp expect actual &&\n+\n+\t# Verify xl_base_# refs have been set correctly.\n+\ttest_cmp_rev xl_base_1 h/1 &&\n+\ttest_cmp_rev xl_base_2 h/10\n+'\n+\n+test_expect_success 'hide branches when branch..no-xl is on' '\n+\tgit checkout branch4 &&\n+\tgit config branch.branch-a.no-xl true &&\n+\tgit config branch.branch2.no-xl true &&\n+\txl >actual &&\n+\n+\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n+\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n+\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n+\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n+\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n+\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n+\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n+\n+\techo \"\\\n+$hashvl1 * 1 committer@example.com [branch3]\n+ | baz6\n+ |\n+$hashvl2 * 2 committer@example.com\n+ |\\ merge2\n+ | |\n+$hashvl3 | | * 3 committer@example.com [HEAD branch4]\n+ | | | baz5\n+ | | |\n+$hashvl4 | | * 4 committer@example.com\n+ | | |\\ merge1\n+ | |/ /\n+ | | /\n+ | |/\n+ |/|\n+$hashvl5 * | 5 committer@example.com\n+ | | not_yet_pushed\n+ | |\n+$hashvl6 | * 6 committer@example.com\n+ |/ baz4\n+ |\n+$hashvl7 * 7 committer@example.com [upstream_repo/branch3]\n+ baz\n+\" >expect &&\n+\ttest_cmp expect actual\n+'\n+\n+test_done\ndiff --git a/xl.c b/xl.c\nnew file mode 100644\nindex 0000000000..539e590f6b\n--- /dev/null\n+++ b/xl.c\n@@ -0,0 +1,485 @@\n+#include \"builtin.h\"\n+#include \"cache.h\"\n+#include \"color.h\"\n+#include \"commit-reach.h\"\n+#include \"config.h\"\n+#include \"oidmap.h\"\n+#include \"ref-filter.h\"\n+#include \"refs.h\"\n+#include \"refs/refs-internal.h\"\n+#include \"remote.h\"\n+#include \"run-command.h\"\n+#include \"strbuf.h\"\n+\n+#include \n+#include \n+#include \n+#include \n+#include \n+#include \n+\n+static void set_ref(\n+\tstruct ref_transaction *ref_tr,\n+\tchar const *name,\n+\tconst struct object_id *oid)\n+{\n+\tstruct strbuf err = STRBUF_INIT;\n+\n+\tif (ref_transaction_update(ref_tr, name, oid, NULL, 0, NULL, &err))\n+\t\tdie(\"%s\", err.buf);\n+\n+\tstrbuf_release(&err);\n+}\n+\n+struct hash_to_ref {\n+\tstruct oidmap_entry e;\n+\n+\tstruct ref_array_item **refs;\n+\tsize_t nr;\n+\tsize_t alloc;\n+};\n+\n+/* An array of ref_array_item's which are not owned by this structure. */\n+struct ref_selection {\n+\tstruct ref_array_item **items;\n+\tsize_t alloc;\n+\tsize_t nr;\n+};\n+\n+static void populate_hash_to_ref_map(\n+\tstruct oidmap *m,\n+\tstruct ref_selection *refs)\n+{\n+\tsize_t ref_i;\n+\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n+\t\tstruct hash_to_ref *h2r;\n+\t\tstruct ref_array_item *ref = refs->items[ref_i];\n+\n+\t\th2r = oidmap_get(m, &ref->objectname);\n+\t\tif (!h2r) {\n+\t\t\th2r = xcalloc(1, sizeof(*h2r));\n+\t\t\toidcpy(&h2r->e.oid, &ref->objectname);\n+\t\t\toidmap_put(m, h2r);\n+\t\t}\n+\t\tALLOC_GROW_BY(h2r->refs, h2r->nr, 1, h2r->alloc);\n+\t\th2r->refs[h2r->nr - 1] = ref;\n+\t}\n+}\n+\n+/*\n+ * Helps invoke `git log` for a certain kind of graph format and process that\n+ * output. One instance of this object lives for the entire invocation of\n+ * `git xl` even if multiple disjoint graphs are included.\n+ */\n+struct log_processing {\n+\tstruct strbuf raw_line;\n+\tstruct strbuf line_buf;\n+\tstruct strbuf line_prefix;\n+\tstruct strbuf sym_refs;\n+\tstruct strbuf tag_name;\n+\n+\tstruct child_process log_proc;\n+\n+\t/* A buffered stream of the output of `git log` */\n+\tFILE *stream;\n+\n+\t/*\n+\t * Number of hashes found and abbreviated since the first graph was\n+\t * started.\n+\t */\n+\tsize_t hash_count;\n+\n+\tunsigned graph_count;\n+\n+\t/*\n+\t * Maps object IDs to hash_to_ref objects which contain all the ref\n+\t * names that ref to the object.\n+\t */\n+\tconst struct oidmap *h2r;\n+\n+\t/*\n+\t * All references that the user desires to be included in a graph. This\n+\t * array may get resorted.\n+\t */\n+\tstruct ref_selection *refs;\n+\n+\t/*\n+\t * Index pointing to the first element that has not been included in a\n+\t * graph yet.\n+\t */\n+\tsize_t ref_i;\n+\n+\t/* Transaction for creating h/# and xl_base(_#) refs. */\n+\tstruct ref_transaction *ref_tr;\n+};\n+\n+#define LOG_PROCESSING_INIT { \\\n+\tSTRBUF_INIT, \\\n+\tSTRBUF_INIT, \\\n+\tSTRBUF_INIT, \\\n+\tSTRBUF_INIT, \\\n+\tSTRBUF_INIT, \\\n+}\n+\n+static void log_processing_finish_proc(struct log_processing *p)\n+{\n+\tint err;\n+\n+\tfclose(p->stream);\n+\tp->stream = NULL;\n+\terr = finish_command(&p->log_proc);\n+\tif (err)\n+\t\tdie(_(\"log failed or could not be terminated: 0x%x\"), err);\n+}\n+\n+static void log_processing_release(struct log_processing *p)\n+{\n+\tif (p->stream)\n+\t\tBUG(\"last log stdout was not closed\");\n+\tstrbuf_release(&p->raw_line);\n+\tstrbuf_release(&p->line_buf);\n+\tstrbuf_release(&p->line_prefix);\n+\tstrbuf_release(&p->sym_refs);\n+\tstrbuf_release(&p->tag_name);\n+}\n+\n+#define XL_HASH_PREFIX \"<{xl_hash}>\"\n+\n+/*\n+ * Begins a `git log` sub process with a subset of the branches requested.\n+ *\n+ * This log invocation shows a graph (using --graph) with full hashes. The\n+ * hashes are prefixed with XL_HASH_PREFIX so they can get easily extracted.\n+ *\n+ * This function also sets the xl_base or xl_base_# ref to the merge base of\n+ * the branches included.\n+ */\n+static int log_processing_start_proc(struct log_processing *p)\n+{\n+\tsize_t ref_i;\n+\tsize_t start_ref_i = p->ref_i;\n+\tsize_t end_ref_i = p->refs->nr;\n+\tstruct commit *merge_base;\n+\n+\tif (p->ref_i == p->refs->nr)\n+\t\treturn 0;\n+\n+\t/*\n+\t * Split the p->refs[] sub array starting at start_ref_i into two\n+\t * sections, re-ordering if needed.\n+\t *\n+\t * The first section contains all commits which share a common ancestor\n+\t * with p->refs->items[start_ref_i]. The second section contains all\n+\t * other commits. In the process, we determine the merge base of the\n+\t * subset. If there are multiple merge bases, we only keep track of one.\n+\t * This is because `git log --graph ` only needs one\n+\t * of the merge bases to intelligently limit the graph size.\n+\t *\n+\t * After the loop is complete, end_ref_i will point to the first item\n+\t * in the second section.\n+\t */\n+\tmerge_base = lookup_commit(\n+\t\tthe_repository, &p->refs->items[start_ref_i]->objectname);\n+\tfor (ref_i = start_ref_i + 1; ref_i < end_ref_i;) {\n+\t\tstruct commit *next = lookup_commit(\n+\t\t\tthe_repository, &p->refs->items[ref_i]->objectname);\n+\t\tstruct commit_list *clist = repo_get_merge_bases(\n+\t\t\tthe_repository, merge_base, next);\n+\n+\t\tif (!clist) {\n+\t\t\t/*\n+\t\t\t * The ref at ref_i does not share a common ancestor\n+\t\t\t * with the refs processed since start_ref_i. Move the\n+\t\t\t * ref at ref_i to the end of the refs array, and move\n+\t\t\t * the item already at the end of the array to ref_i.\n+\t\t\t * This allows us to postpone processing this orphan\n+\t\t\t * branch until the next `git log` invocation.\n+\t\t\t */\n+\t\t\tstruct ref_array_item *tmp = p->refs->items[ref_i];\n+\t\t\tp->refs->items[ref_i] = p->refs->items[--end_ref_i];\n+\t\t\tp->refs->items[end_ref_i] = tmp;\n+\t\t} else {\n+\t\t\tmerge_base = clist->item;\n+\t\t\tfree_commit_list(clist);\n+\t\t\tref_i++;\n+\t\t}\n+\t}\n+\n+\tp->graph_count++;\n+\tif (!start_ref_i && end_ref_i == p->refs->nr) {\n+\t\t/* Only a single log graph in this invocation of `git xl`. */\n+\t\tset_ref(p->ref_tr, \"xl_base\", &merge_base->object.oid);\n+\t} else {\n+\t\t/* Multiple log graphs - use a counter to disambiguate bases. */\n+\t\tstruct strbuf xl_base_ref_name = STRBUF_INIT;\n+\t\tstrbuf_addf(&xl_base_ref_name, \"xl_base_%u\", p->graph_count);\n+\t\tset_ref(p->ref_tr, xl_base_ref_name.buf,\n+\t\t\t&merge_base->object.oid);\n+\t\tstrbuf_release(&xl_base_ref_name);\n+\t}\n+\n+\tchild_process_init(&p->log_proc);\n+\tp->log_proc.git_cmd = 1;\n+\tp->log_proc.out = -1;\n+\tp->log_proc.no_stdin = 1;\n+\n+\targv_array_pushl(&p->log_proc.args, \"log\", \"--graph\", NULL);\n+\targv_array_pushf(&p->log_proc.args, \"--color=%s\",\n+\t\t\t want_color(GIT_COLOR_UNKNOWN) ? \"always\" : \"never\");\n+\targv_array_push(&p->log_proc.args,\n+\t\t\t\"--format=format:\" XL_HASH_PREFIX \"%H %ce\\n%s\\n \");\n+\tfor (ref_i = start_ref_i; ref_i < end_ref_i; ref_i++)\n+\t\targv_array_push(\n+\t\t\t&p->log_proc.args, p->refs->items[ref_i]->refname);\n+\targv_array_pushf(&p->log_proc.args, \"^%s^@\",\n+\t\t\t oid_to_hex(&merge_base->object.oid));\n+\targv_array_push(&p->log_proc.args, \"--\");\n+\n+\tif (start_command(&p->log_proc))\n+\t\tdie(_(\"cannot start log\"));\n+\n+\tp->stream = xfdopen(p->log_proc.out, \"r\");\n+\n+\tp->ref_i = end_ref_i;\n+\n+\treturn 1;\n+}\n+\n+static const char *color_on(const char *c)\n+{\n+\treturn want_color(GIT_COLOR_UNKNOWN) ? c : \"\";\n+}\n+\n+static const char *color_off(void)\n+{\n+\treturn want_color(GIT_COLOR_UNKNOWN) ? \"\\e[0m\" : \"\";\n+}\n+\n+static void maybe_format_symrefs(\n+\tstruct strbuf *sym_refs,\n+\tstruct oidmap const *h2r,\n+\tconst struct object_id *oid)\n+{\n+\tstruct hash_to_ref const *h2r_entry;\n+\tsize_t ref_i;\n+\n+\th2r_entry = oidmap_get(h2r, oid);\n+\n+\tif (!h2r_entry)\n+\t\treturn;\n+\n+\tstrbuf_addf(sym_refs, \" %s[\", color_on(\"\\e[1m\"));\n+\n+\tfor (ref_i = 0; ref_i < h2r_entry->nr; ref_i++) {\n+\t\tchar *shortened_ref = shorten_unambiguous_ref(\n+\t\t\th2r_entry->refs[ref_i]->refname, /*strict=*/1);\n+\n+\t\tif (ref_i)\n+\t\t\tstrbuf_addch(sym_refs, ' ');\n+\n+\t\tstrbuf_addstr(sym_refs, shortened_ref);\n+\t\tfree(shortened_ref);\n+\t}\n+\n+\tstrbuf_addf(sym_refs, \"]%s\", color_off());\n+}\n+\n+static int process_log_line(struct log_processing *p)\n+{\n+\tconst char *in;\n+\tsize_t hash_prefix_len = strlen(XL_HASH_PREFIX);\n+\n+\tstrbuf_reset(&p->raw_line);\n+\tstrbuf_reset(&p->line_buf);\n+\tstrbuf_reset(&p->line_prefix);\n+\tstrbuf_reset(&p->sym_refs);\n+\tstrbuf_reset(&p->tag_name);\n+\n+\tif (strbuf_getline_lf(&p->raw_line, p->stream) == EOF)\n+\t\treturn 0;\n+\n+\tin = p->raw_line.buf;\n+\n+\twhile (*in) {\n+\t\tstruct object_id oid;\n+\t\tconst char *after_hash;\n+\n+\t\tif (p->line_prefix.len ||\n+\t\t strncmp(XL_HASH_PREFIX, in, hash_prefix_len) ||\n+\t\t parse_oid_hex(in + hash_prefix_len, &oid, &after_hash)) {\n+\t\t\tstrbuf_addch(&p->line_buf, *in++);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\tp->hash_count++;\n+\t\tstrbuf_addf(&p->line_buf,\n+\t\t\t \"%s %ld %s\",\n+\t\t\t color_on(\"\\e[48;5;213m\\e[30m\"),\n+\t\t\t p->hash_count,\n+\t\t\t color_off());\n+\n+\t\tstrbuf_addf(&p->line_prefix,\n+\t\t\t \"%s%.8s%s\",\n+\t\t\t color_on(\"\\e[38;5;147m\"),\n+\t\t\t in + hash_prefix_len,\n+\t\t\t color_off());\n+\t\tin = after_hash;\n+\n+\t\tstrbuf_addf(&p->tag_name, \"h/%ld\", p->hash_count);\n+\t\tset_ref(p->ref_tr, p->tag_name.buf, &oid);\n+\n+\t\tmaybe_format_symrefs(&p->sym_refs, p->h2r, &oid);\n+\t}\n+\n+\tfprintf(stdout, \"%8s %s%s\\n\",\n+\t\tp->line_prefix.buf,\n+\t\tp->line_buf.buf,\n+\t\tp->sym_refs.buf);\n+\n+\treturn 1;\n+}\n+\n+static void empty_hash_to_ref_map(struct oidmap *m)\n+{\n+\tstruct oidmap_iter i;\n+\tstruct hash_to_ref *h2r;\n+\toidmap_iter_init(m, &i);\n+\n+\twhile ((h2r = oidmap_iter_next(&i)) != NULL) {\n+\t\tFREE_AND_NULL(h2r->refs);\n+\t\th2r->alloc = 0;\n+\t\th2r->nr = 0;\n+\t}\n+}\n+\n+static int add_ref(struct ref_array *refs, const char *name)\n+{\n+\tstruct object_id oid;\n+\tsize_t ref_i;\n+\n+\t/* If we already have the ref, don't add it again. */\n+\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n+\t\tif (!strcmp(refs->items[ref_i]->refname, name))\n+\t\t\treturn 0;\n+\t}\n+\n+\tif (get_oid(name, &oid))\n+\t\tdie(\"unknown object: %s\", name);\n+\tref_array_push(refs, name, &oid);\n+\t\n+\treturn 1;\n+}\n+\n+static void select_ref(\n+\tstruct ref_selection *ref_sel,\n+\tstruct ref_array *refs,\n+\tsize_t ref_i)\n+{\n+\tALLOC_GROW_BY(ref_sel->items, ref_sel->nr, 1, ref_sel->alloc);\n+\tref_sel->items[ref_sel->nr - 1] = refs->items[ref_i];\n+}\n+\n+static void populate_branch_args(\n+\tstruct ref_array *refs,\n+\tstruct ref_selection *ref_sel,\n+\tconst char **argv)\n+{\n+\tstruct ref_filter filter = {0};\n+\tsize_t ref_i;\n+\tsize_t ref_i_end;\n+\tstruct strbuf no_xl_config_key = STRBUF_INIT;\n+\n+\tfilter.name_patterns = argv;\n+\tfilter_refs(refs, &filter, FILTER_REFS_BRANCHES);\n+\n+\tref_i_end = refs->nr;\n+\n+\t/* Add upstream branches of each branch. */\n+\tfor (ref_i = 0; ref_i < ref_i_end; ref_i++) {\n+\t\tstruct branch *branch = branch_get(refs->items[ref_i]->refname);\n+\t\tchar *short_name;\n+\t\tconst char *upstream;\n+\t\tint no_xl = 0;\n+\n+\t\tif (!branch) {\n+\t\t\t/*\n+\t\t\t * Not actually a branch, but might be HEAD. Select this\n+\t\t\t * ref for display.\n+\t\t\t */\n+\t\t\tselect_ref(ref_sel, refs, ref_i);\n+\t\t\tcontinue;\n+\t\t}\n+\n+\t\t/*\n+\t\t * Do not show the branch or its upstream if user configured\n+\t\t * branch..no-xl = true\n+\t\t */\n+\t\tshort_name = shorten_unambiguous_ref(\n+\t\t\tbranch->name, /*strict=*/1);\n+\t\tstrbuf_reset(&no_xl_config_key);\n+\t\tstrbuf_addf(&no_xl_config_key, \"branch.%s.no-xl\", short_name);\n+\t\tFREE_AND_NULL(short_name);\n+\n+\t\tif (!git_config_get_bool(no_xl_config_key.buf, &no_xl) && no_xl)\n+\t\t\tcontinue;\n+\n+\t\tselect_ref(ref_sel, refs, ref_i);\n+\t\tupstream = branch_get_upstream(branch, NULL);\n+\n+\t\t/*\n+\t\t * Add the upstream branch if it has not been added as the\n+\t\t * upstream of some other local branch.\n+\t\t */\n+\t\tif (upstream && add_ref(refs, upstream))\n+\t\t\tselect_ref(ref_sel, refs, refs->nr - 1);\n+\t}\n+\n+\tstrbuf_release(&no_xl_config_key);\n+}\n+\n+int cmd_xl(int argc, const char **argv, const char *prefx)\n+{\n+\tstruct oidmap hash_to_ref_map = OIDMAP_INIT;\n+\tstruct ref_selection ref_sel = {0};\n+\tstruct ref_array refs = {0};\n+\tstruct strbuf ref_tr_err = STRBUF_INIT;\n+\tstruct ref_transaction *ref_tr;\n+\tstruct log_processing log_processing = LOG_PROCESSING_INIT;\n+\n+\tgit_config(git_color_config, NULL);\n+\n+\t/*\n+\t * Add HEAD first. This way, if we output multiple graphs, the first\n+\t * one will include the currently checked-out ref.\n+\t */\n+\tadd_ref(&refs, \"HEAD\");\n+\n+\tpopulate_branch_args(&refs, &ref_sel, argv + 1);\n+\n+\toidmap_init(&hash_to_ref_map, 16);\n+\tpopulate_hash_to_ref_map(&hash_to_ref_map, &ref_sel);\n+\n+\tif (!(ref_tr = ref_transaction_begin(&ref_tr_err)))\n+\t\tdie(\"%s\", ref_tr_err.buf);\n+\n+\tlog_processing.h2r = &hash_to_ref_map;\n+\tlog_processing.ref_tr = ref_tr;\n+\tlog_processing.refs = &ref_sel;\n+\twhile (log_processing_start_proc(&log_processing)) {\n+\t\twhile (process_log_line(&log_processing)) {}\n+\t\tlog_processing_finish_proc(&log_processing);\n+\t}\n+\n+\tif (ref_transaction_commit(ref_tr, &ref_tr_err))\n+\t\tdie(\"%s\", ref_tr_err.buf);\n+\n+\tempty_hash_to_ref_map(&hash_to_ref_map);\n+\toidmap_free(&hash_to_ref_map, 1);\n+\tref_array_clear(&refs);\n+\tref_transaction_free(ref_tr);\n+\tstrbuf_release(&ref_tr_err);\n+\tlog_processing_release(&log_processing);\n+\tFREE_AND_NULL(ref_sel.items);\n+\n+\treturn 0;\n+}\n-- \n2.19.0.605.g01d371f741-goog\n\n"},{"id":"385194","messageId":"20191031003929.GA22855@google.com","threadId":"52145","inReplyTo":"20191029003023.122196-1-matvore@google.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Emily Shaffer","fromEmail":"emilyshaffer@google.com","sentAt":"2019-10-31T00:39:29Z","receivedAt":"2019-10-31T00:39:42Z","isPatch":false,"sender":{"key":"nasamuffin@google.com","avatar":"https://avatars.githubusercontent.com/u/1606826?v=4"},"body":"On Mon, Oct 28, 2019 at 05:30:23PM -0700, Matthew DeVore wrote:\n> From: Matthew DeVore \n\nHi Matthew,\n\nGood to hear from you. One comment - the subject of your mail is \"[RFC]\"\nbut I think folks are used to receiving mails with RFC patches if the\nsubject line is formatted like it comes out of 'git format-patch' - that\nis, [RFC PATCH].\n\n> \n> \"git xl\" shows a graph of recent history, including all existing\n> branches (unless flagged with a config option) and their upstream\n> counterparts. It is named such because it is easy to type and the\n> letter \"x\" looks like a small graph.\n\nFor me, that's not a very compelling reason to name something, and the\nonly command with such a cryptic name in Git that I can think of is 'git\nam'. (mv, gc, rm, and p4 are somewhat self explanatory, and everything\nelse besides 'gitk' is named with a full word.)\n\n> \n> Like \"git branch\" it supports filtering the branches shown via\n> positional arguments.\n> \n> Besides just showing the graph, it also associates refs with all visible\n> commits with names in the form of \"h/#\" where # is an incrementing\n> index. After showing the graph, these refs can be used to ergonomically\n> invoke some follow-up command like rebase or diff.\n\nIt looks like there's a decent amount of this commit message which\nreally ought to be a note to the reviewers instead. Everything above the\n'---' goes into the commit message; everything below it will get\nscrubbed when the patch is applied, so you can give more casual notes\nthere - for example this paragraph, as well as \"Omissions I might/will\nfix\".\n\n> The test cases show non-trivial output which can be used to get an idea\n> for what the command is good for, though it doesn't capture the\n> coloring.\n> \n> The primary goals of this command are:\n> \n> a) deduce what the user wants to see based on what they haven't pushed\n> upstream yet\n> b) show the active branches spatially rather than as a linear list (as\n> in \"git branch\")\n> c) allow the user to easily refer to commits that appeared in the\n> output\n> \n> I considered making the h/# tags stable across invocations such that a\n> particular hash will only be tagged with a different number if ~100\n> other hashes are tagged since the hash was last tagged. I didn't\n> actually implement it this way, instead opting for always re-numbering\n> the hashes on each invocation. This means the hash number is\n> predictable based on the position the hash appears in the output, which\n> is probably better that encouraging users to memorize hash numbers (or\n> use them in scripts!).\n\nIf you're worried about folks using something like this in a script (and\nI would be, given that it's dynamically assigning nicknames to hashes)\nthen you probably ought to mark it as a porcelain command in\ncommand-list.txt.\n\n> \n> Omissions I might/will fix depending on feedback:\n> \n> a) rather than show HEAD in the graph, show when\n> possible (i.e. \"[]\" rather than \"[HEAD master]\").\n> \n> b) don't parse output from `git log` but instead do everything\n> in-process.\n> \n> c) documentation\n\nSorry not to review the rest of the diff today; I'll try to get to it\nsometime soon.\n\n - Emily\n\n> ---\n> Makefile | 1 +\n> builtin.h | 1 +\n> git.c | 1 +\n> t/t4400-xl.sh | 270 ++++++++++++++++++++++++++++\n> xl.c | 485 ++++++++++++++++++++++++++++++++++++++++++++++++++\n> 5 files changed, 758 insertions(+)\n> create mode 100755 t/t4400-xl.sh\n> create mode 100644 xl.c\n> \n> diff --git a/Makefile b/Makefile\n> index 03b800da0c..491661f848 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1022,20 +1022,21 @@ LIB_OBJS += varint.o\n> LIB_OBJS += version.o\n> LIB_OBJS += versioncmp.o\n> LIB_OBJS += walker.o\n> LIB_OBJS += wildmatch.o\n> LIB_OBJS += worktree.o\n> LIB_OBJS += wrapper.o\n> LIB_OBJS += write-or-die.o\n> LIB_OBJS += ws.o\n> LIB_OBJS += wt-status.o\n> LIB_OBJS += xdiff-interface.o\n> +LIB_OBJS += xl.o\n> LIB_OBJS += zlib.o\n> \n> BUILTIN_OBJS += builtin/add.o\n> BUILTIN_OBJS += builtin/am.o\n> BUILTIN_OBJS += builtin/annotate.o\n> BUILTIN_OBJS += builtin/apply.o\n> BUILTIN_OBJS += builtin/archive.o\n> BUILTIN_OBJS += builtin/bisect--helper.o\n> BUILTIN_OBJS += builtin/blame.o\n> BUILTIN_OBJS += builtin/branch.o\n> diff --git a/builtin.h b/builtin.h\n> index 5cf5df69f7..568d09cf7f 100644\n> --- a/builtin.h\n> +++ b/builtin.h\n> @@ -241,16 +241,17 @@ int cmd_update_server_info(int argc, const char **argv, const char *prefix);\n> int cmd_upload_archive(int argc, const char **argv, const char *prefix);\n> int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix);\n> int cmd_upload_pack(int argc, const char **argv, const char *prefix);\n> int cmd_var(int argc, const char **argv, const char *prefix);\n> int cmd_verify_commit(int argc, const char **argv, const char *prefix);\n> int cmd_verify_tag(int argc, const char **argv, const char *prefix);\n> int cmd_version(int argc, const char **argv, const char *prefix);\n> int cmd_whatchanged(int argc, const char **argv, const char *prefix);\n> int cmd_worktree(int argc, const char **argv, const char *prefix);\n> int cmd_write_tree(int argc, const char **argv, const char *prefix);\n> +int cmd_xl(int argc, const char **argv, const char *prefix);\n> int cmd_verify_pack(int argc, const char **argv, const char *prefix);\n> int cmd_show_ref(int argc, const char **argv, const char *prefix);\n> int cmd_pack_refs(int argc, const char **argv, const char *prefix);\n> int cmd_replace(int argc, const char **argv, const char *prefix);\n> \n> #endif\n> diff --git a/git.c b/git.c\n> index ce6ab0ece2..4a1da83a7e 100644\n> --- a/git.c\n> +++ b/git.c\n> @@ -594,20 +594,21 @@ static struct cmd_struct commands[] = {\n> \t{ \"upload-archive--writer\", cmd_upload_archive_writer, NO_PARSEOPT },\n> \t{ \"upload-pack\", cmd_upload_pack },\n> \t{ \"var\", cmd_var, RUN_SETUP_GENTLY | NO_PARSEOPT },\n> \t{ \"verify-commit\", cmd_verify_commit, RUN_SETUP },\n> \t{ \"verify-pack\", cmd_verify_pack },\n> \t{ \"verify-tag\", cmd_verify_tag, RUN_SETUP },\n> \t{ \"version\", cmd_version },\n> \t{ \"whatchanged\", cmd_whatchanged, RUN_SETUP },\n> \t{ \"worktree\", cmd_worktree, RUN_SETUP | NO_PARSEOPT },\n> \t{ \"write-tree\", cmd_write_tree, RUN_SETUP },\n> +\t{ \"xl\", cmd_xl, RUN_SETUP },\n> };\n> \n> static struct cmd_struct *get_builtin(const char *s)\n> {\n> \tint i;\n> \tfor (i = 0; i < ARRAY_SIZE(commands); i++) {\n> \t\tstruct cmd_struct *p = commands + i;\n> \t\tif (!strcmp(s, p->cmd))\n> \t\t\treturn p;\n> \t}\n> diff --git a/t/t4400-xl.sh b/t/t4400-xl.sh\n> new file mode 100755\n> index 0000000000..f6e35bd4da\n> --- /dev/null\n> +++ b/t/t4400-xl.sh\n> @@ -0,0 +1,270 @@\n> +#!/bin/sh\n> +\n> +test_description='git xl'\n> +. ./test-lib.sh\n> +\n> +xl () {\n> +\tgit xl \"$@\" >actual_raw &&\n> +\tsed -e \"s/ *$//\" actual_raw\n> +}\n> +\n> +test_expect_success 'basic' '\n> +\ttest_commit foo &&\n> +\tgit checkout -b branch2 &&\n> +\ttest_commit bar &&\n> +\n> +\txl >actual &&\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch2]\n> + | bar\n> + |\n> +$hashvl2 * 2 committer@example.com [master]\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'specify ref names' '\n> +\txl master >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD]\n> + | bar\n> + |\n> +$hashvl2 * 2 committer@example.com [master]\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'deduce graph base' '\n> +\tgit checkout -b branch3 master &&\n> +\ttest_commit baz &&\n> +\tgit branch -d master &&\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\txl_base=$(git rev-parse xl_base | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | baz\n> + |\n> +$hashvl2 | * 2 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$xl_base * 3 committer@example.com\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show upstream branch' '\n> +\tgit init --bare upstream_repo.git &&\n> +\tgit remote add upstream_repo upstream_repo.git &&\n> +\n> +\tgit push -u upstream_repo HEAD &&\n> +\tgit branch --set-upstream-to=upstream_repo/branch3 &&\n> +\ttest_commit not_yet_pushed &&\n> +\n> +\t# Exclude branch2 by requesting at least one other ref explicitly.\n> +\txl branch3 >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | not_yet_pushed\n> + |\n> +$hashvl2 * 2 committer@example.com [upstream_repo/branch3]\n> + baz\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'de-dupe upstream branches' '\n> +\tgit checkout -b branch4 upstream_repo/branch3 &&\n> +\ttest_commit baz4 &&\n> +\n> +\t# Make sure we do not show the same upstream branch name twice\n> +\t# even though two local branches share the same upstream branch.\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch4]\n> + | baz4\n> + |\n> +$hashvl2 | * 2 committer@example.com [branch3]\n> + |/ not_yet_pushed\n> + |\n> +$hashvl3 * 3 committer@example.com [upstream_repo/branch3]\n> + | baz\n> + |\n> +$hashvl4 | * 4 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$hashvl5 * 5 committer@example.com\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'multiple merge bases' '\n> +\tgit merge -m merge1 branch3 &&\n> +\ttest_commit baz5 &&\n> +\n> +\tgit checkout branch3 &&\n> +\tgit merge -m merge2 h/1 &&\n> +\ttest_commit baz6 &&\n> +\n> +\tgit branch --unset-upstream branch3 &&\n> +\txl branch3 branch4 >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | baz6\n> + |\n> +$hashvl2 * 2 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl3 | | * 3 committer@example.com [branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl4 | | * 4 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl5 * | 5 committer@example.com\n> + / not_yet_pushed\n> + |\n> +$hashvl6 * 6 committer@example.com\n> + baz4\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'orphan branches' '\n> +\t# If there are some branches to display which do not have a common\n> +\t# ancestor with the other branches, we show them in a separate graph.\n> +\tgit checkout --orphan branch-a h/6 &&\n> +\tgit commit -m baz7 &&\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n> +\thashvl8=$(git rev-parse h/8 | test_copy_bytes 8) &&\n> +\thashvl9=$(git rev-parse h/9 | test_copy_bytes 8) &&\n> +\thashv10=$(git rev-parse h/10 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch-a]\n> + baz7\n> +\n> +$hashvl2 * 2 committer@example.com [branch3]\n> + | baz6\n> + |\n> +$hashvl3 * 3 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl4 | | * 4 committer@example.com [branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl5 | | * 5 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl6 * | 6 committer@example.com\n> + | | not_yet_pushed\n> + | |\n> +$hashvl7 | * 7 committer@example.com\n> + |/ baz4\n> + |\n> +$hashvl8 * 8 committer@example.com\n> + | baz\n> + |\n> +$hashvl9 | * 9 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$hashv10 * 10 committer@example.com\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual &&\n> +\n> +\t# Verify xl_base_# refs have been set correctly.\n> +\ttest_cmp_rev xl_base_1 h/1 &&\n> +\ttest_cmp_rev xl_base_2 h/10\n> +'\n> +\n> +test_expect_success 'hide branches when branch..no-xl is on' '\n> +\tgit checkout branch4 &&\n> +\tgit config branch.branch-a.no-xl true &&\n> +\tgit config branch.branch2.no-xl true &&\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [branch3]\n> + | baz6\n> + |\n> +$hashvl2 * 2 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl3 | | * 3 committer@example.com [HEAD branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl4 | | * 4 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl5 * | 5 committer@example.com\n> + | | not_yet_pushed\n> + | |\n> +$hashvl6 | * 6 committer@example.com\n> + |/ baz4\n> + |\n> +$hashvl7 * 7 committer@example.com [upstream_repo/branch3]\n> + baz\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n> diff --git a/xl.c b/xl.c\n> new file mode 100644\n> index 0000000000..539e590f6b\n> --- /dev/null\n> +++ b/xl.c\n> @@ -0,0 +1,485 @@\n> +#include \"builtin.h\"\n> +#include \"cache.h\"\n> +#include \"color.h\"\n> +#include \"commit-reach.h\"\n> +#include \"config.h\"\n> +#include \"oidmap.h\"\n> +#include \"ref-filter.h\"\n> +#include \"refs.h\"\n> +#include \"refs/refs-internal.h\"\n> +#include \"remote.h\"\n> +#include \"run-command.h\"\n> +#include \"strbuf.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static void set_ref(\n> +\tstruct ref_transaction *ref_tr,\n> +\tchar const *name,\n> +\tconst struct object_id *oid)\n> +{\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\n> +\tif (ref_transaction_update(ref_tr, name, oid, NULL, 0, NULL, &err))\n> +\t\tdie(\"%s\", err.buf);\n> +\n> +\tstrbuf_release(&err);\n> +}\n> +\n> +struct hash_to_ref {\n> +\tstruct oidmap_entry e;\n> +\n> +\tstruct ref_array_item **refs;\n> +\tsize_t nr;\n> +\tsize_t alloc;\n> +};\n> +\n> +/* An array of ref_array_item's which are not owned by this structure. */\n> +struct ref_selection {\n> +\tstruct ref_array_item **items;\n> +\tsize_t alloc;\n> +\tsize_t nr;\n> +};\n> +\n> +static void populate_hash_to_ref_map(\n> +\tstruct oidmap *m,\n> +\tstruct ref_selection *refs)\n> +{\n> +\tsize_t ref_i;\n> +\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n> +\t\tstruct hash_to_ref *h2r;\n> +\t\tstruct ref_array_item *ref = refs->items[ref_i];\n> +\n> +\t\th2r = oidmap_get(m, &ref->objectname);\n> +\t\tif (!h2r) {\n> +\t\t\th2r = xcalloc(1, sizeof(*h2r));\n> +\t\t\toidcpy(&h2r->e.oid, &ref->objectname);\n> +\t\t\toidmap_put(m, h2r);\n> +\t\t}\n> +\t\tALLOC_GROW_BY(h2r->refs, h2r->nr, 1, h2r->alloc);\n> +\t\th2r->refs[h2r->nr - 1] = ref;\n> +\t}\n> +}\n> +\n> +/*\n> + * Helps invoke `git log` for a certain kind of graph format and process that\n> + * output. One instance of this object lives for the entire invocation of\n> + * `git xl` even if multiple disjoint graphs are included.\n> + */\n> +struct log_processing {\n> +\tstruct strbuf raw_line;\n> +\tstruct strbuf line_buf;\n> +\tstruct strbuf line_prefix;\n> +\tstruct strbuf sym_refs;\n> +\tstruct strbuf tag_name;\n> +\n> +\tstruct child_process log_proc;\n> +\n> +\t/* A buffered stream of the output of `git log` */\n> +\tFILE *stream;\n> +\n> +\t/*\n> +\t * Number of hashes found and abbreviated since the first graph was\n> +\t * started.\n> +\t */\n> +\tsize_t hash_count;\n> +\n> +\tunsigned graph_count;\n> +\n> +\t/*\n> +\t * Maps object IDs to hash_to_ref objects which contain all the ref\n> +\t * names that ref to the object.\n> +\t */\n> +\tconst struct oidmap *h2r;\n> +\n> +\t/*\n> +\t * All references that the user desires to be included in a graph. This\n> +\t * array may get resorted.\n> +\t */\n> +\tstruct ref_selection *refs;\n> +\n> +\t/*\n> +\t * Index pointing to the first element that has not been included in a\n> +\t * graph yet.\n> +\t */\n> +\tsize_t ref_i;\n> +\n> +\t/* Transaction for creating h/# and xl_base(_#) refs. */\n> +\tstruct ref_transaction *ref_tr;\n> +};\n> +\n> +#define LOG_PROCESSING_INIT { \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +}\n> +\n> +static void log_processing_finish_proc(struct log_processing *p)\n> +{\n> +\tint err;\n> +\n> +\tfclose(p->stream);\n> +\tp->stream = NULL;\n> +\terr = finish_command(&p->log_proc);\n> +\tif (err)\n> +\t\tdie(_(\"log failed or could not be terminated: 0x%x\"), err);\n> +}\n> +\n> +static void log_processing_release(struct log_processing *p)\n> +{\n> +\tif (p->stream)\n> +\t\tBUG(\"last log stdout was not closed\");\n> +\tstrbuf_release(&p->raw_line);\n> +\tstrbuf_release(&p->line_buf);\n> +\tstrbuf_release(&p->line_prefix);\n> +\tstrbuf_release(&p->sym_refs);\n> +\tstrbuf_release(&p->tag_name);\n> +}\n> +\n> +#define XL_HASH_PREFIX \"<{xl_hash}>\"\n> +\n> +/*\n> + * Begins a `git log` sub process with a subset of the branches requested.\n> + *\n> + * This log invocation shows a graph (using --graph) with full hashes. The\n> + * hashes are prefixed with XL_HASH_PREFIX so they can get easily extracted.\n> + *\n> + * This function also sets the xl_base or xl_base_# ref to the merge base of\n> + * the branches included.\n> + */\n> +static int log_processing_start_proc(struct log_processing *p)\n> +{\n> +\tsize_t ref_i;\n> +\tsize_t start_ref_i = p->ref_i;\n> +\tsize_t end_ref_i = p->refs->nr;\n> +\tstruct commit *merge_base;\n> +\n> +\tif (p->ref_i == p->refs->nr)\n> +\t\treturn 0;\n> +\n> +\t/*\n> +\t * Split the p->refs[] sub array starting at start_ref_i into two\n> +\t * sections, re-ordering if needed.\n> +\t *\n> +\t * The first section contains all commits which share a common ancestor\n> +\t * with p->refs->items[start_ref_i]. The second section contains all\n> +\t * other commits. In the process, we determine the merge base of the\n> +\t * subset. If there are multiple merge bases, we only keep track of one.\n> +\t * This is because `git log --graph ` only needs one\n> +\t * of the merge bases to intelligently limit the graph size.\n> +\t *\n> +\t * After the loop is complete, end_ref_i will point to the first item\n> +\t * in the second section.\n> +\t */\n> +\tmerge_base = lookup_commit(\n> +\t\tthe_repository, &p->refs->items[start_ref_i]->objectname);\n> +\tfor (ref_i = start_ref_i + 1; ref_i < end_ref_i;) {\n> +\t\tstruct commit *next = lookup_commit(\n> +\t\t\tthe_repository, &p->refs->items[ref_i]->objectname);\n> +\t\tstruct commit_list *clist = repo_get_merge_bases(\n> +\t\t\tthe_repository, merge_base, next);\n> +\n> +\t\tif (!clist) {\n> +\t\t\t/*\n> +\t\t\t * The ref at ref_i does not share a common ancestor\n> +\t\t\t * with the refs processed since start_ref_i. Move the\n> +\t\t\t * ref at ref_i to the end of the refs array, and move\n> +\t\t\t * the item already at the end of the array to ref_i.\n> +\t\t\t * This allows us to postpone processing this orphan\n> +\t\t\t * branch until the next `git log` invocation.\n> +\t\t\t */\n> +\t\t\tstruct ref_array_item *tmp = p->refs->items[ref_i];\n> +\t\t\tp->refs->items[ref_i] = p->refs->items[--end_ref_i];\n> +\t\t\tp->refs->items[end_ref_i] = tmp;\n> +\t\t} else {\n> +\t\t\tmerge_base = clist->item;\n> +\t\t\tfree_commit_list(clist);\n> +\t\t\tref_i++;\n> +\t\t}\n> +\t}\n> +\n> +\tp->graph_count++;\n> +\tif (!start_ref_i && end_ref_i == p->refs->nr) {\n> +\t\t/* Only a single log graph in this invocation of `git xl`. */\n> +\t\tset_ref(p->ref_tr, \"xl_base\", &merge_base->object.oid);\n> +\t} else {\n> +\t\t/* Multiple log graphs - use a counter to disambiguate bases. */\n> +\t\tstruct strbuf xl_base_ref_name = STRBUF_INIT;\n> +\t\tstrbuf_addf(&xl_base_ref_name, \"xl_base_%u\", p->graph_count);\n> +\t\tset_ref(p->ref_tr, xl_base_ref_name.buf,\n> +\t\t\t&merge_base->object.oid);\n> +\t\tstrbuf_release(&xl_base_ref_name);\n> +\t}\n> +\n> +\tchild_process_init(&p->log_proc);\n> +\tp->log_proc.git_cmd = 1;\n> +\tp->log_proc.out = -1;\n> +\tp->log_proc.no_stdin = 1;\n> +\n> +\targv_array_pushl(&p->log_proc.args, \"log\", \"--graph\", NULL);\n> +\targv_array_pushf(&p->log_proc.args, \"--color=%s\",\n> +\t\t\t want_color(GIT_COLOR_UNKNOWN) ? \"always\" : \"never\");\n> +\targv_array_push(&p->log_proc.args,\n> +\t\t\t\"--format=format:\" XL_HASH_PREFIX \"%H %ce\\n%s\\n \");\n> +\tfor (ref_i = start_ref_i; ref_i < end_ref_i; ref_i++)\n> +\t\targv_array_push(\n> +\t\t\t&p->log_proc.args, p->refs->items[ref_i]->refname);\n> +\targv_array_pushf(&p->log_proc.args, \"^%s^@\",\n> +\t\t\t oid_to_hex(&merge_base->object.oid));\n> +\targv_array_push(&p->log_proc.args, \"--\");\n> +\n> +\tif (start_command(&p->log_proc))\n> +\t\tdie(_(\"cannot start log\"));\n> +\n> +\tp->stream = xfdopen(p->log_proc.out, \"r\");\n> +\n> +\tp->ref_i = end_ref_i;\n> +\n> +\treturn 1;\n> +}\n> +\n> +static const char *color_on(const char *c)\n> +{\n> +\treturn want_color(GIT_COLOR_UNKNOWN) ? c : \"\";\n> +}\n> +\n> +static const char *color_off(void)\n> +{\n> +\treturn want_color(GIT_COLOR_UNKNOWN) ? \"\\e[0m\" : \"\";\n> +}\n> +\n> +static void maybe_format_symrefs(\n> +\tstruct strbuf *sym_refs,\n> +\tstruct oidmap const *h2r,\n> +\tconst struct object_id *oid)\n> +{\n> +\tstruct hash_to_ref const *h2r_entry;\n> +\tsize_t ref_i;\n> +\n> +\th2r_entry = oidmap_get(h2r, oid);\n> +\n> +\tif (!h2r_entry)\n> +\t\treturn;\n> +\n> +\tstrbuf_addf(sym_refs, \" %s[\", color_on(\"\\e[1m\"));\n> +\n> +\tfor (ref_i = 0; ref_i < h2r_entry->nr; ref_i++) {\n> +\t\tchar *shortened_ref = shorten_unambiguous_ref(\n> +\t\t\th2r_entry->refs[ref_i]->refname, /*strict=*/1);\n> +\n> +\t\tif (ref_i)\n> +\t\t\tstrbuf_addch(sym_refs, ' ');\n> +\n> +\t\tstrbuf_addstr(sym_refs, shortened_ref);\n> +\t\tfree(shortened_ref);\n> +\t}\n> +\n> +\tstrbuf_addf(sym_refs, \"]%s\", color_off());\n> +}\n> +\n> +static int process_log_line(struct log_processing *p)\n> +{\n> +\tconst char *in;\n> +\tsize_t hash_prefix_len = strlen(XL_HASH_PREFIX);\n> +\n> +\tstrbuf_reset(&p->raw_line);\n> +\tstrbuf_reset(&p->line_buf);\n> +\tstrbuf_reset(&p->line_prefix);\n> +\tstrbuf_reset(&p->sym_refs);\n> +\tstrbuf_reset(&p->tag_name);\n> +\n> +\tif (strbuf_getline_lf(&p->raw_line, p->stream) == EOF)\n> +\t\treturn 0;\n> +\n> +\tin = p->raw_line.buf;\n> +\n> +\twhile (*in) {\n> +\t\tstruct object_id oid;\n> +\t\tconst char *after_hash;\n> +\n> +\t\tif (p->line_prefix.len ||\n> +\t\t strncmp(XL_HASH_PREFIX, in, hash_prefix_len) ||\n> +\t\t parse_oid_hex(in + hash_prefix_len, &oid, &after_hash)) {\n> +\t\t\tstrbuf_addch(&p->line_buf, *in++);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tp->hash_count++;\n> +\t\tstrbuf_addf(&p->line_buf,\n> +\t\t\t \"%s %ld %s\",\n> +\t\t\t color_on(\"\\e[48;5;213m\\e[30m\"),\n> +\t\t\t p->hash_count,\n> +\t\t\t color_off());\n> +\n> +\t\tstrbuf_addf(&p->line_prefix,\n> +\t\t\t \"%s%.8s%s\",\n> +\t\t\t color_on(\"\\e[38;5;147m\"),\n> +\t\t\t in + hash_prefix_len,\n> +\t\t\t color_off());\n> +\t\tin = after_hash;\n> +\n> +\t\tstrbuf_addf(&p->tag_name, \"h/%ld\", p->hash_count);\n> +\t\tset_ref(p->ref_tr, p->tag_name.buf, &oid);\n> +\n> +\t\tmaybe_format_symrefs(&p->sym_refs, p->h2r, &oid);\n> +\t}\n> +\n> +\tfprintf(stdout, \"%8s %s%s\\n\",\n> +\t\tp->line_prefix.buf,\n> +\t\tp->line_buf.buf,\n> +\t\tp->sym_refs.buf);\n> +\n> +\treturn 1;\n> +}\n> +\n> +static void empty_hash_to_ref_map(struct oidmap *m)\n> +{\n> +\tstruct oidmap_iter i;\n> +\tstruct hash_to_ref *h2r;\n> +\toidmap_iter_init(m, &i);\n> +\n> +\twhile ((h2r = oidmap_iter_next(&i)) != NULL) {\n> +\t\tFREE_AND_NULL(h2r->refs);\n> +\t\th2r->alloc = 0;\n> +\t\th2r->nr = 0;\n> +\t}\n> +}\n> +\n> +static int add_ref(struct ref_array *refs, const char *name)\n> +{\n> +\tstruct object_id oid;\n> +\tsize_t ref_i;\n> +\n> +\t/* If we already have the ref, don't add it again. */\n> +\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n> +\t\tif (!strcmp(refs->items[ref_i]->refname, name))\n> +\t\t\treturn 0;\n> +\t}\n> +\n> +\tif (get_oid(name, &oid))\n> +\t\tdie(\"unknown object: %s\", name);\n> +\tref_array_push(refs, name, &oid);\n> +\t\n> +\treturn 1;\n> +}\n> +\n> +static void select_ref(\n> +\tstruct ref_selection *ref_sel,\n> +\tstruct ref_array *refs,\n> +\tsize_t ref_i)\n> +{\n> +\tALLOC_GROW_BY(ref_sel->items, ref_sel->nr, 1, ref_sel->alloc);\n> +\tref_sel->items[ref_sel->nr - 1] = refs->items[ref_i];\n> +}\n> +\n> +static void populate_branch_args(\n> +\tstruct ref_array *refs,\n> +\tstruct ref_selection *ref_sel,\n> +\tconst char **argv)\n> +{\n> +\tstruct ref_filter filter = {0};\n> +\tsize_t ref_i;\n> +\tsize_t ref_i_end;\n> +\tstruct strbuf no_xl_config_key = STRBUF_INIT;\n> +\n> +\tfilter.name_patterns = argv;\n> +\tfilter_refs(refs, &filter, FILTER_REFS_BRANCHES);\n> +\n> +\tref_i_end = refs->nr;\n> +\n> +\t/* Add upstream branches of each branch. */\n> +\tfor (ref_i = 0; ref_i < ref_i_end; ref_i++) {\n> +\t\tstruct branch *branch = branch_get(refs->items[ref_i]->refname);\n> +\t\tchar *short_name;\n> +\t\tconst char *upstream;\n> +\t\tint no_xl = 0;\n> +\n> +\t\tif (!branch) {\n> +\t\t\t/*\n> +\t\t\t * Not actually a branch, but might be HEAD. Select this\n> +\t\t\t * ref for display.\n> +\t\t\t */\n> +\t\t\tselect_ref(ref_sel, refs, ref_i);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Do not show the branch or its upstream if user configured\n> +\t\t * branch..no-xl = true\n> +\t\t */\n> +\t\tshort_name = shorten_unambiguous_ref(\n> +\t\t\tbranch->name, /*strict=*/1);\n> +\t\tstrbuf_reset(&no_xl_config_key);\n> +\t\tstrbuf_addf(&no_xl_config_key, \"branch.%s.no-xl\", short_name);\n> +\t\tFREE_AND_NULL(short_name);\n> +\n> +\t\tif (!git_config_get_bool(no_xl_config_key.buf, &no_xl) && no_xl)\n> +\t\t\tcontinue;\n> +\n> +\t\tselect_ref(ref_sel, refs, ref_i);\n> +\t\tupstream = branch_get_upstream(branch, NULL);\n> +\n> +\t\t/*\n> +\t\t * Add the upstream branch if it has not been added as the\n> +\t\t * upstream of some other local branch.\n> +\t\t */\n> +\t\tif (upstream && add_ref(refs, upstream))\n> +\t\t\tselect_ref(ref_sel, refs, refs->nr - 1);\n> +\t}\n> +\n> +\tstrbuf_release(&no_xl_config_key);\n> +}\n> +\n> +int cmd_xl(int argc, const char **argv, const char *prefx)\n> +{\n> +\tstruct oidmap hash_to_ref_map = OIDMAP_INIT;\n> +\tstruct ref_selection ref_sel = {0};\n> +\tstruct ref_array refs = {0};\n> +\tstruct strbuf ref_tr_err = STRBUF_INIT;\n> +\tstruct ref_transaction *ref_tr;\n> +\tstruct log_processing log_processing = LOG_PROCESSING_INIT;\n> +\n> +\tgit_config(git_color_config, NULL);\n> +\n> +\t/*\n> +\t * Add HEAD first. This way, if we output multiple graphs, the first\n> +\t * one will include the currently checked-out ref.\n> +\t */\n> +\tadd_ref(&refs, \"HEAD\");\n> +\n> +\tpopulate_branch_args(&refs, &ref_sel, argv + 1);\n> +\n> +\toidmap_init(&hash_to_ref_map, 16);\n> +\tpopulate_hash_to_ref_map(&hash_to_ref_map, &ref_sel);\n> +\n> +\tif (!(ref_tr = ref_transaction_begin(&ref_tr_err)))\n> +\t\tdie(\"%s\", ref_tr_err.buf);\n> +\n> +\tlog_processing.h2r = &hash_to_ref_map;\n> +\tlog_processing.ref_tr = ref_tr;\n> +\tlog_processing.refs = &ref_sel;\n> +\twhile (log_processing_start_proc(&log_processing)) {\n> +\t\twhile (process_log_line(&log_processing)) {}\n> +\t\tlog_processing_finish_proc(&log_processing);\n> +\t}\n> +\n> +\tif (ref_transaction_commit(ref_tr, &ref_tr_err))\n> +\t\tdie(\"%s\", ref_tr_err.buf);\n> +\n> +\tempty_hash_to_ref_map(&hash_to_ref_map);\n> +\toidmap_free(&hash_to_ref_map, 1);\n> +\tref_array_clear(&refs);\n> +\tref_transaction_free(ref_tr);\n> +\tstrbuf_release(&ref_tr_err);\n> +\tlog_processing_release(&log_processing);\n> +\tFREE_AND_NULL(ref_sel.items);\n> +\n> +\treturn 0;\n> +}\n> -- \n> 2.19.0.605.g01d371f741-goog\n> \n"},{"id":"385202","messageId":"nycvar.QRO.7.76.6.1910310851300.46@tvgsbejvaqbjf.bet","threadId":"52145","inReplyTo":"20191031003929.GA22855@google.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-31T08:26:48Z","receivedAt":"2019-10-31T08:27:13Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Wed, 30 Oct 2019, Emily Shaffer wrote:\n\n> On Mon, Oct 28, 2019 at 05:30:23PM -0700, Matthew DeVore wrote:\n> > From: Matthew DeVore \n>\n> Good to hear from you. One comment - the subject of your mail is \"[RFC]\"\n> but I think folks are used to receiving mails with RFC patches if the\n> subject line is formatted like it comes out of 'git format-patch' - that\n> is, [RFC PATCH].\n>\n> >\n> > \"git xl\" shows a graph of recent history, including all existing\n> > branches (unless flagged with a config option) and their upstream\n> > counterparts. It is named such because it is easy to type and the\n> > letter \"x\" looks like a small graph.\n>\n> For me, that's not a very compelling reason to name something, and the\n> only command with such a cryptic name in Git that I can think of is 'git\n> am'. (mv, gc, rm, and p4 are somewhat self explanatory, and everything\n> else besides 'gitk' is named with a full word.)\n\nam stands for \"apply mbox\", and I think that the only reason it is not\ncalled `git apply-mbox` is that the Linux maintainer uses it a lot and\nwanted to save on keystrokes.\n\nHaving said that, I do agree that `xl` is not a good name for this. It\nis neither intuitive, nor is it particularly easy to type (on a\nUS-English keyboard, the `x` and the `l` key are far apart), and to add\ninsult to injury, _any_ two-letter command is likely to shadow\nalready-existing aliases that users might have installed locally.\n\nBesides, from the description it sounds to me that this would be better\nimplemented as a new mode for, say, `show-branch` (I could imagine e.g.\n`git show-branch --unpushed` to be a good name for this operation).\n\n> > Like \"git branch\" it supports filtering the branches shown via\n> > positional arguments.\n\n... or `git branch --show-unpushed`...\n\n> > Besides just showing the graph, it also associates refs with all visible\n> > commits with names in the form of \"h/#\" where # is an incrementing\n> > index. After showing the graph, these refs can be used to ergonomically\n> > invoke some follow-up command like rebase or diff.\n>\n> It looks like there's a decent amount of this commit message which\n> really ought to be a note to the reviewers instead. Everything above the\n> '---' goes into the commit message; everything below it will get\n> scrubbed when the patch is applied, so you can give more casual notes\n> there - for example this paragraph, as well as \"Omissions I might/will\n> fix\".\n\nIn addition, I would think that the introduction of ephemeral refs\nshould deserve its own patch. Such ephemeral refs might come in handy\nfor more things than just `xl` (or whatever better name we find).\n\nThe design of such ephemeral refs is thoroughly interesting, too.\n\nOne very obvious question is whether you want these refs to be\nworktree-specific or not. I would tend to answer \"yes\" to that question.\n\nFurther, another obvious question is what to do with those refs after a\nwhile. They are _clearly_ intended to be ephemeral, i.e. they should\njust vanish after a reasonably short time. Which raises the question:\nwhat is \"reasonably short\" in this context? We would probably want to\ncome up with a good default and then offer a config setting to change\nit.\n\nAnother important aspect is the naming. The naming schema you chose\n(`h/`) is short-and-sweet, and might very well be in use\nalready, for totally different purposes. It would be a really good idea\nto open that schema to allow for avoiding clashes with already-existing\nrefs.\n\nA better alternative might be to choose a naming schema that cannot\nclash with existing refs because it would not make for valid ref names.\nI had a look at the ref name validation, and `^` might be a\nbetter naming schema to begin with: `^1` is not a valid ref name, for\nexample.\n\nSide note: why `h/`? I really tried to think about possible motivations\nand came up empty.\n\nAnother aspect that I think should be considered: why limit these\nephemeral refs to `git xl`? I cannot count how often I look through\nsome `git log -- `\nto find a certain commit and then need to reference it. I usually move\nmy hand to move the mouse pointer and double click, then Shift-Insert\n(which is awkward on this here keyboard because Insert is Fn+Delete, so\nI cannot do that with one hand), and I usually wish for some better way.\n\nA better way might be to introduce an option for generating and\ndisplaying such ephemeral refs, in my case it would be good to have a\nconfig setting to do that automatically for every `git log` call that\nuses the pager, i.e. is interactive.\n\nFinally, I could imagine that in this context, we would love to have\nrefs that are purely intended for interactive use, and therefore it\nwould make sense to try to bind them to the process ID of the process\ncalling `git`, i.e. the interactive shell. That way, when I have two\nterminal windows, they would \"own\" their separate ephemeral refs.\n\n> > The test cases show non-trivial output which can be used to get an idea\n> > for what the command is good for, though it doesn't capture the\n> > coloring.\n> >\n> > The primary goals of this command are:\n> >\n> > a) deduce what the user wants to see based on what they haven't pushed\n> > upstream yet\n> > b) show the active branches spatially rather than as a linear list (as\n> > in \"git branch\")\n> > c) allow the user to easily refer to commits that appeared in the\n> > output\n> >\n> > I considered making the h/# tags stable across invocations such that a\n> > particular hash will only be tagged with a different number if ~100\n> > other hashes are tagged since the hash was last tagged. I didn't\n> > actually implement it this way, instead opting for always re-numbering\n> > the hashes on each invocation. This means the hash number is\n> > predictable based on the position the hash appears in the output, which\n> > is probably better that encouraging users to memorize hash numbers (or\n> > use them in scripts!).\n>\n> If you're worried about folks using something like this in a script (and\n> I would be, given that it's dynamically assigning nicknames to hashes)\n> then you probably ought to mark it as a porcelain command in\n> command-list.txt.\n\nI would like to caution against targeting scripts with this. It is too\neasy for two concurrently running scripts to stumble over each other.\n\nScripts should use safer methods that already exist, like grabbing the\nhash while looking for a specific pattern (`sed`'s hold space comes to\nmind).\n\nCiao,\nDscho\n"},{"id":"385210","messageId":"nycvar.QRO.7.76.6.1910310929130.46@tvgsbejvaqbjf.bet","threadId":"52145","inReplyTo":"20191029003023.122196-1-matvore@google.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-10-31T10:16:18Z","receivedAt":"2019-10-31T10:16:45Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matthew,\n\nOn Mon, 28 Oct 2019, Matthew DeVore wrote:\n\n> From: Matthew DeVore \n>\n> \"git xl\" shows a graph of recent history, including all existing\n> branches (unless flagged with a config option) and their upstream\n> counterparts. It is named such because it is easy to type and the\n> letter \"x\" looks like a small graph.\n\nI like to see first paragraphs of commit messages that peak my curiosity\nor that excite me about what is to come. Alan Alda's advice for\nscientists comes to mind: tell a story, and every story begins with a\nrelatable struggle, a struggle that the audience can allude to.\n\nIn this instance, the first paragraph could be improved in that respect,\nI think.\n\nIn my worktrees, I usually have a few dozen branches that are in flight,\nsome of them demoted to lower priority after a brief period of intense\nactivity, and it would be really nice to have a command to see quickly\nwhere I left off, in order of local activity, filtered by the branches\nthat are unpushed, showing their relationships (if any).\n\nAny commit message with a first paragraph that describes such a\nscenario, and then says that its patch is going to help me with it, will\n_immediately_ have my full attention.\n\n> Like \"git branch\" it supports filtering the branches shown via\n> positional arguments.\n\nFollowing up by an example would make this sentence easier to\nunderstand.\n\nAnd before even talking about options it supports, it might be a good\nidea to illustrate the command with an example output.\n\n> Besides just showing the graph, it also associates refs with all visible\n> commits with names in the form of \"h/#\" where # is an incrementing\n> index. After showing the graph, these refs can be used to ergonomically\n> invoke some follow-up command like rebase or diff.\n\nAs I mentioned in my previous reply to Emily's answer, I think that this\nwould be a useful thing to have in `git log`, and it should therefore be\nsplit out into its own patch, maybe even its own patch series.\n\n> The test cases show non-trivial output which can be used to get an idea\n> for what the command is good for, though it doesn't capture the\n> coloring.\n\nIf the coloring is so helpful, then it should be tested, too, via\n`test_decode_color`.\n\n> The primary goals of this command are:\n>\n> a) deduce what the user wants to see based on what they haven't pushed\n> upstream yet\n> b) show the active branches spatially rather than as a linear list (as\n> in \"git branch\")\n> c) allow the user to easily refer to commits that appeared in the\n> output\n\nAha! This motivation for the patch should have come a lot earlier.\n\nIt still is a bit unclear to me what \"spatially rather than as a linear\nlist\" means. Are you referring to the output of `git show-branch` vs the\noutput of `git branch`?\n\n> I considered making the h/# tags stable across invocations such that a\n> particular hash will only be tagged with a different number if ~100\n> other hashes are tagged since the hash was last tagged. I didn't\n> actually implement it this way, instead opting for always re-numbering\n> the hashes on each invocation. This means the hash number is\n> predictable based on the position the hash appears in the output, which\n> is probably better that encouraging users to memorize hash numbers (or\n> use them in scripts!).\n\nAgain, as I mentioned in my previous reply, this is not a thing for\nscripts. Scripts to not have fingers, they don't need to type, and they\ncertainly do not tire of long, precise commit hashes.\n\nThe fact that the design calls for overwriting previously-generated refs\nmakes this really dangerous: what if the refs it overwrites were not\nactually generated by this command, but were carefully generated\nelsewhere?\n\n> Omissions I might/will fix depending on feedback:\n>\n> a) rather than show HEAD in the graph, show when\n> possible (i.e. \"[]\" rather than \"[HEAD master]\").\n\nI don't quite understand. I guess this concern requires the reader to be\nalready familiar with the usage of this command, which would require me\nto find a revision to which I can apply this patch, then compile locally\nand run it. That's not what I expect of an RFC. Could you at least give\nan example output here?\n\nHaving said that, I have the suspicion that you are talking about\ndecorating the commits with the applicable refs? The prior art would be\n`(HEAD -> master)` as generated by `git log --decorate`.\n\n> b) don't parse output from `git log` but instead do everything\n> in-process.\n\nApart from the ephemeral refs (which would probably be useful in `git\nlog` to begin with, as I already stated), it would seem that at least\nsome of the ideas in this new command might be implemented better as new\nmodes in the log-tree machinery.\n\n> c) documentation\n\nIndeed.\n\n> diff --git a/t/t4400-xl.sh b/t/t4400-xl.sh\n> new file mode 100755\n> index 0000000000..f6e35bd4da\n> --- /dev/null\n> +++ b/t/t4400-xl.sh\n> @@ -0,0 +1,270 @@\n> +#!/bin/sh\n> +\n> +test_description='git xl'\n> +. ./test-lib.sh\n> +\n> +xl () {\n> +\tgit xl \"$@\" >actual_raw &&\n\n\nWould it not make more sense for the command itself to right-trim its\noutput already?\n\n> +}\n> +\n> +test_expect_success 'basic' '\n> +\ttest_commit foo &&\n> +\tgit checkout -b branch2 &&\n> +\ttest_commit bar &&\n> +\n> +\txl >actual &&\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n\nIt looks a bit fragile to use the generated `h/*` refs to verify the\noutput, and I am not sure that you want to force the abbreviation that\nway rather than use the native `--short=8` option. It would be a better\nidea to use something like\n\n\th1=$(git rev-parse --short=8 bar) &&\n\th2=$(git rev-parse --short=8 foo) &&\n\nand after checking the output, verifying _independently_ that the `h/*`\nrefs were generated correctly, e.g.\n\n\ttest_cmp_rev $h1 h/1 &&\n\ttest_cmp_rev $h2 h/2\n\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch2]\n> + | bar\n> + |\n> +$hashvl2 * 2 committer@example.com [master]\n> + foo\n> +\" >expect &&\n\nI don't think that we ever use multi-line `echo` elsewhere in the test\nsuite, instead we seem to use `cat >expect <<-EOF &&` a lot. Maybe it\nwould be better to follow that convention than to invent a competing\none?\n\n> +\ttest_cmp expect actual\n\nOkay, so what is done _a lot_ in this test script is to generate the\noutput of `xl`, to write out the expected output, and then compare them.\nLet's dry that up, following the example of `t/t3430-rebase-merges.sh`'\n`test_cmp_graph` function:\n\ntest_cmp_xl () {\n\tcat >expect &&\n\tgit xl \"$@\" >output &&\n\tsed \"s/ *$//\" output.trimmed &&\n\ttest_cmp expect output.trimmed\n}\n\nThere. Much conciser, and you can even leave the right-trimming to the\nscript.\n\n> +'\n> +\n> +test_expect_success 'specify ref names' '\n> +\txl master >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n\nCorrect me if I am wrong, but it seems that these lines are repeated an\nawful lot. Besides, as I said, they do not test precisely enough: if\n`git xl` would display the wrong hashes here, and then store the same\nwrong hashes in `h/*`, the test script would still pass.\n\nA better way would be to use the _actually_ expected hashes, and to\nassign them in an initial `setup` test case e.g.\n\ntest_expect_success 'setup' '\n\ttest_commit foo &&\n\tfoo=$(git rev-parse --short=8 foo) &&\n\tgit switch -c branch2 &&\n\ttest_commit bar &&\n\tbar=$(git rev-parse --short=8 bar)\n'\n\nThis initial 'setup' test case is a well-established convention in Git's\ntest suite.\n\nOf course, an even cleverer approach would make use of the fact that\n`test_commit` uses the commit message as tag name, too, and let\n`test_cmp_xl` _generate_ those hashes.\n\nOnly `baz7` seems to be committed via `git commit` directly and would\nneed a `git tag baz7`. It would need a `test_tick`, too, anyway...\n\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD]\n> + | bar\n> + |\n> +$hashvl2 * 2 committer@example.com [master]\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'deduce graph base' '\n> +\tgit checkout -b branch3 master &&\n> +\ttest_commit baz &&\n> +\tgit branch -d master &&\n> +\txl >actual &&\n> +\n\nLogically, the empty line should be between the set-up and the test,\ni.e. _before_ the `xl` line.\n\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\txl_base=$(git rev-parse xl_base | test_copy_bytes 8) &&\n\nWait, where does this `xl_base` come from?\n\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | baz\n> + |\n> +$hashvl2 | * 2 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$xl_base * 3 committer@example.com\n\nWhoa. Why is this not called `h/3`? I must have read the commit message\nwrong.\n\n*goes-back-and-checks*\n\nNo, the commit message suggests that an incremental index (which is by\nthe way not a good terminology here, as \"index\" already means something\n_very_ different in Git, \"counter\" would be a better term to use) is\nused. Not `xl_base`.\n\nAnd I think it would make more sense to stick with the `x/*` schema,\ntoo.\n\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'show upstream branch' '\n> +\tgit init --bare upstream_repo.git &&\n> +\tgit remote add upstream_repo upstream_repo.git &&\n> +\n> +\tgit push -u upstream_repo HEAD &&\n> +\tgit branch --set-upstream-to=upstream_repo/branch3 &&\n> +\ttest_commit not_yet_pushed &&\n> +\n> +\t# Exclude branch2 by requesting at least one other ref explicitly.\n> +\txl branch3 >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | not_yet_pushed\n> + |\n> +$hashvl2 * 2 committer@example.com [upstream_repo/branch3]\n> + baz\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'de-dupe upstream branches' '\n> +\tgit checkout -b branch4 upstream_repo/branch3 &&\n> +\ttest_commit baz4 &&\n> +\n> +\t# Make sure we do not show the same upstream branch name twice\n> +\t# even though two local branches share the same upstream branch.\n\nWait, why?\n\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch4]\n> + | baz4\n> + |\n> +$hashvl2 | * 2 committer@example.com [branch3]\n> + |/ not_yet_pushed\n> + |\n> +$hashvl3 * 3 committer@example.com [upstream_repo/branch3]\n\nOkay, it is shown here. Which is what I would expect. Why would it be\nshown twice? Was there a bug in a patch iteration that was not sent to\nthe mailing list? I could understand that, but I would still phrase the\ncomment above \"Make sure that each upstream branch name is shown only\nonce, even though multiple local branches share it as upstream branch.\"\n\n> + | baz\n> + |\n> +$hashvl4 | * 4 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$hashvl5 * 5 committer@example.com\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'multiple merge bases' '\n> +\tgit merge -m merge1 branch3 &&\n> +\ttest_commit baz5 &&\n> +\n> +\tgit checkout branch3 &&\n> +\tgit merge -m merge2 h/1 &&\n> +\ttest_commit baz6 &&\n> +\n> +\tgit branch --unset-upstream branch3 &&\n> +\txl branch3 branch4 >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch3]\n> + | baz6\n> + |\n> +$hashvl2 * 2 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl3 | | * 3 committer@example.com [branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl4 | | * 4 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl5 * | 5 committer@example.com\n> + / not_yet_pushed\n> + |\n> +$hashvl6 * 6 committer@example.com\n> + baz4\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_expect_success 'orphan branches' '\n> +\t# If there are some branches to display which do not have a common\n> +\t# ancestor with the other branches, we show them in a separate graph.\n> +\tgit checkout --orphan branch-a h/6 &&\n> +\tgit commit -m baz7 &&\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytes 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n> +\thashvl8=$(git rev-parse h/8 | test_copy_bytes 8) &&\n> +\thashvl9=$(git rev-parse h/9 | test_copy_bytes 8) &&\n> +\thashv10=$(git rev-parse h/10 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [HEAD branch-a]\n> + baz7\n> +\n> +$hashvl2 * 2 committer@example.com [branch3]\n> + | baz6\n> + |\n> +$hashvl3 * 3 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl4 | | * 4 committer@example.com [branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl5 | | * 5 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl6 * | 6 committer@example.com\n> + | | not_yet_pushed\n> + | |\n> +$hashvl7 | * 7 committer@example.com\n> + |/ baz4\n> + |\n> +$hashvl8 * 8 committer@example.com\n> + | baz\n> + |\n> +$hashvl9 | * 9 committer@example.com [branch2]\n> + |/ bar\n> + |\n> +$hashv10 * 10 committer@example.com\n> + foo\n> +\" >expect &&\n> +\ttest_cmp expect actual &&\n> +\n> +\t# Verify xl_base_# refs have been set correctly.\n> +\ttest_cmp_rev xl_base_1 h/1 &&\n> +\ttest_cmp_rev xl_base_2 h/10\n> +'\n> +\n> +test_expect_success 'hide branches when branch..no-xl is on' '\n> +\tgit checkout branch4 &&\n> +\tgit config branch.branch-a.no-xl true &&\n> +\tgit config branch.branch2.no-xl true &&\n> +\txl >actual &&\n> +\n> +\thashvl1=$(git rev-parse h/1 | test_copy_bytes 8) &&\n> +\thashvl2=$(git rev-parse h/2 | test_copy_bytes 8) &&\n> +\thashvl3=$(git rev-parse h/3 | test_copy_bytES 8) &&\n> +\thashvl4=$(git rev-parse h/4 | test_copy_bytes 8) &&\n> +\thashvl5=$(git rev-parse h/5 | test_copy_bytes 8) &&\n> +\thashvl6=$(git rev-parse h/6 | test_copy_bytes 8) &&\n> +\thashvl7=$(git rev-parse h/7 | test_copy_bytes 8) &&\n> +\n> +\techo \"\\\n> +$hashvl1 * 1 committer@example.com [branch3]\n> + | baz6\n> + |\n> +$hashvl2 * 2 committer@example.com\n> + |\\ merge2\n> + | |\n> +$hashvl3 | | * 3 committer@example.com [HEAD branch4]\n> + | | | baz5\n> + | | |\n> +$hashvl4 | | * 4 committer@example.com\n> + | | |\\ merge1\n> + | |/ /\n> + | | /\n> + | |/\n> + |/|\n> +$hashvl5 * | 5 committer@example.com\n> + | | not_yet_pushed\n> + | |\n> +$hashvl6 | * 6 committer@example.com\n> + |/ baz4\n> + |\n> +$hashvl7 * 7 committer@example.com [upstream_repo/branch3]\n> + baz\n> +\" >expect &&\n> +\ttest_cmp expect actual\n> +'\n> +\n> +test_done\n\nAfter reading through this script, I cannot fail to notice that the\ncommitter is always the same, and repeated a gazillion times. It might\nbe more readable to use a shorter name, or to inject the email address\nautomatically in `test_cmp_xl`. Dunno.\n\nIn any case, there is a lot of room for DRYing up this test script.\n\n> diff --git a/xl.c b/xl.c\n> new file mode 100644\n> index 0000000000..539e590f6b\n> --- /dev/null\n> +++ b/xl.c\n> @@ -0,0 +1,485 @@\n> +#include \"builtin.h\"\n> +#include \"cache.h\"\n> +#include \"color.h\"\n> +#include \"commit-reach.h\"\n> +#include \"config.h\"\n> +#include \"oidmap.h\"\n> +#include \"ref-filter.h\"\n> +#include \"refs.h\"\n> +#include \"refs/refs-internal.h\"\n> +#include \"remote.h\"\n> +#include \"run-command.h\"\n> +#include \"strbuf.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +static void set_ref(\n> +\tstruct ref_transaction *ref_tr,\n\nThis is not how Git's source code is formatted. Please do not introduce\na new, contradicting convention.\n\n> +\tchar const *name,\n> +\tconst struct object_id *oid)\n> +{\n> +\tstruct strbuf err = STRBUF_INIT;\n> +\n> +\tif (ref_transaction_update(ref_tr, name, oid, NULL, 0, NULL, &err))\n> +\t\tdie(\"%s\", err.buf);\n> +\n> +\tstrbuf_release(&err);\n> +}\n> +\n> +struct hash_to_ref {\n> +\tstruct oidmap_entry e;\n> +\n> +\tstruct ref_array_item **refs;\n> +\tsize_t nr;\n> +\tsize_t alloc;\n> +};\n> +\n> +/* An array of ref_array_item's which are not owned by this structure. */\n> +struct ref_selection {\n> +\tstruct ref_array_item **items;\n> +\tsize_t alloc;\n> +\tsize_t nr;\n> +};\n> +\n> +static void populate_hash_to_ref_map(\n> +\tstruct oidmap *m,\n\nWe could spell it out instead of using a single letter: it is a `map`.\n\n> +\tstruct ref_selection *refs)\n> +{\n> +\tsize_t ref_i;\n\nWhy not just `i`? Why complicating things?\n\n> +\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n> +\t\tstruct hash_to_ref *h2r;\n> +\t\tstruct ref_array_item *ref = refs->items[ref_i];\n> +\n> +\t\th2r = oidmap_get(m, &ref->objectname);\n> +\t\tif (!h2r) {\n> +\t\t\th2r = xcalloc(1, sizeof(*h2r));\n> +\t\t\toidcpy(&h2r->e.oid, &ref->objectname);\n> +\t\t\toidmap_put(m, h2r);\n> +\t\t}\n> +\t\tALLOC_GROW_BY(h2r->refs, h2r->nr, 1, h2r->alloc);\n> +\t\th2r->refs[h2r->nr - 1] = ref;\n\nQuite honestly, I would find it easier to read like it is done\nelsewhere: use `ALLOC_GROW(... nr + 1 ...)` and then `...[nr++] = ...`.\n\nI know, it is done this way _once_, in `list-objects-filter-options.c`,\nbut in the way I suggested twice in `alias.c`, once in `alloc.c`, once\nin `apply.c`, and the list is actually quite long so I won't bore you\nwith the rest.\n\n> +\t}\n> +}\n> +\n> +/*\n> + * Helps invoke `git log` for a certain kind of graph format and process that\n> + * output. One instance of this object lives for the entire invocation of\n> + * `git xl` even if multiple disjoint graphs are included.\n> + */\n> +struct log_processing {\n> +\tstruct strbuf raw_line;\n> +\tstruct strbuf line_buf;\n> +\tstruct strbuf line_prefix;\n> +\tstruct strbuf sym_refs;\n> +\tstruct strbuf tag_name;\n> +\n> +\tstruct child_process log_proc;\n> +\n> +\t/* A buffered stream of the output of `git log` */\n> +\tFILE *stream;\n\nIs it really worth the complexity to read from the stream, rather than\nusing `capture_command()`?\n\n> +\n> +\t/*\n> +\t * Number of hashes found and abbreviated since the first graph was\n> +\t * started.\n> +\t */\n> +\tsize_t hash_count;\n> +\n> +\tunsigned graph_count;\n> +\n> +\t/*\n> +\t * Maps object IDs to hash_to_ref objects which contain all the ref\n> +\t * names that ref to the object.\n> +\t */\n> +\tconst struct oidmap *h2r;\n> +\n> +\t/*\n> +\t * All references that the user desires to be included in a graph. This\n> +\t * array may get resorted.\n> +\t */\n> +\tstruct ref_selection *refs;\n> +\n> +\t/*\n> +\t * Index pointing to the first element that has not been included in a\n> +\t * graph yet.\n> +\t */\n> +\tsize_t ref_i;\n> +\n> +\t/* Transaction for creating h/# and xl_base(_#) refs. */\n> +\tstruct ref_transaction *ref_tr;\n> +};\n> +\n> +#define LOG_PROCESSING_INIT { \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +\tSTRBUF_INIT, \\\n> +}\n> +\n> +static void log_processing_finish_proc(struct log_processing *p)\n> +{\n> +\tint err;\n> +\n> +\tfclose(p->stream);\n> +\tp->stream = NULL;\n> +\terr = finish_command(&p->log_proc);\n> +\tif (err)\n> +\t\tdie(_(\"log failed or could not be terminated: 0x%x\"), err);\n> +}\n> +\n> +static void log_processing_release(struct log_processing *p)\n> +{\n> +\tif (p->stream)\n> +\t\tBUG(\"last log stdout was not closed\");\n> +\tstrbuf_release(&p->raw_line);\n> +\tstrbuf_release(&p->line_buf);\n> +\tstrbuf_release(&p->line_prefix);\n> +\tstrbuf_release(&p->sym_refs);\n> +\tstrbuf_release(&p->tag_name);\n> +}\n> +\n> +#define XL_HASH_PREFIX \"<{xl_hash}>\"\n> +\n> +/*\n> + * Begins a `git log` sub process with a subset of the branches requested.\n> + *\n> + * This log invocation shows a graph (using --graph) with full hashes. The\n> + * hashes are prefixed with XL_HASH_PREFIX so they can get easily extracted.\n\nSince you are using the output using `--graph`, I agree that it is\nbetter to capture and post-process the output of `git log`, at least for\nnow.\n\n> + *\n> + * This function also sets the xl_base or xl_base_# ref to the merge base of\n> + * the branches included.\n> + */\n> +static int log_processing_start_proc(struct log_processing *p)\n> +{\n> +\tsize_t ref_i;\n\nWhat's with these unnecessary `ref_` prefixes? If there is no other `i`\nto be confused with, let's use `i`, plain and simple.\n\n> +\tsize_t start_ref_i = p->ref_i;\n> +\tsize_t end_ref_i = p->refs->nr;\n> +\tstruct commit *merge_base;\n> +\n> +\tif (p->ref_i == p->refs->nr)\n> +\t\treturn 0;\n> +\n> +\t/*\n> +\t * Split the p->refs[] sub array starting at start_ref_i into two\n> +\t * sections, re-ordering if needed.\n> +\t *\n> +\t * The first section contains all commits which share a common ancestor\n> +\t * with p->refs->items[start_ref_i]. The second section contains all\n> +\t * other commits. In the process, we determine the merge base of the\n> +\t * subset. If there are multiple merge bases, we only keep track of one.\n> +\t * This is because `git log --graph ` only needs one\n> +\t * of the merge bases to intelligently limit the graph size.\n> +\t *\n> +\t * After the loop is complete, end_ref_i will point to the first item\n> +\t * in the second section.\n> +\t */\n> +\tmerge_base = lookup_commit(\n\nAgain, please don't invent your own formatting rules that contradict the\nexisting source code's convention.\n\n> +\t\tthe_repository, &p->refs->items[start_ref_i]->objectname);\n> +\tfor (ref_i = start_ref_i + 1; ref_i < end_ref_i;) {\n> +\t\tstruct commit *next = lookup_commit(\n> +\t\t\tthe_repository, &p->refs->items[ref_i]->objectname);\n> +\t\tstruct commit_list *clist = repo_get_merge_bases(\n\nI could imagine that `merge_bases` would be a splendid name for what is\nnow called `clist`.\n\n> +\t\t\tthe_repository, merge_base, next);\n> +\n> +\t\tif (!clist) {\n> +\t\t\t/*\n> +\t\t\t * The ref at ref_i does not share a common ancestor\n> +\t\t\t * with the refs processed since start_ref_i. Move the\n> +\t\t\t * ref at ref_i to the end of the refs array, and move\n> +\t\t\t * the item already at the end of the array to ref_i.\n> +\t\t\t * This allows us to postpone processing this orphan\n> +\t\t\t * branch until the next `git log` invocation.\n> +\t\t\t */\n> +\t\t\tstruct ref_array_item *tmp = p->refs->items[ref_i];\n> +\t\t\tp->refs->items[ref_i] = p->refs->items[--end_ref_i];\n> +\t\t\tp->refs->items[end_ref_i] = tmp;\n\nIt would probably be a lot clearer to write\n\n\t\t\tSWAP(p->refs->items[ref_i], p->refs->items[end_ref_i]);\n\t\t\tend_ref_i--;\n\n> +\t\t} else {\n> +\t\t\tmerge_base = clist->item;\n> +\t\t\tfree_commit_list(clist);\n> +\t\t\tref_i++;\n> +\t\t}\n> +\t}\n> +\n> +\tp->graph_count++;\n> +\tif (!start_ref_i && end_ref_i == p->refs->nr) {\n> +\t\t/* Only a single log graph in this invocation of `git xl`. */\n> +\t\tset_ref(p->ref_tr, \"xl_base\", &merge_base->object.oid);\n\nSo that's where the `xl_base` comes from.\n\nI still would _much_ prefer the merge base to be labeled with just yet\nanother `x/*` ref. _Much_.\n\n> +\t} else {\n> +\t\t/* Multiple log graphs - use a counter to disambiguate bases. */\n> +\t\tstruct strbuf xl_base_ref_name = STRBUF_INIT;\n> +\t\tstrbuf_addf(&xl_base_ref_name, \"xl_base_%u\", p->graph_count);\n> +\t\tset_ref(p->ref_tr, xl_base_ref_name.buf,\n> +\t\t\t&merge_base->object.oid);\n> +\t\tstrbuf_release(&xl_base_ref_name);\n> +\t}\n> +\n> +\tchild_process_init(&p->log_proc);\n> +\tp->log_proc.git_cmd = 1;\n> +\tp->log_proc.out = -1;\n> +\tp->log_proc.no_stdin = 1;\n> +\n> +\targv_array_pushl(&p->log_proc.args, \"log\", \"--graph\", NULL);\n> +\targv_array_pushf(&p->log_proc.args, \"--color=%s\",\n> +\t\t\t want_color(GIT_COLOR_UNKNOWN) ? \"always\" : \"never\");\n> +\targv_array_push(&p->log_proc.args,\n> +\t\t\t\"--format=format:\" XL_HASH_PREFIX \"%H %ce\\n%s\\n \");\n\nI wonder why we don't just use `%h` here. And `%d` for the decoration.\n\n> +\tfor (ref_i = start_ref_i; ref_i < end_ref_i; ref_i++)\n> +\t\targv_array_push(\n> +\t\t\t&p->log_proc.args, p->refs->items[ref_i]->refname);\n> +\targv_array_pushf(&p->log_proc.args, \"^%s^@\",\n> +\t\t\t oid_to_hex(&merge_base->object.oid));\n\nWouldn't it make more sense to use `^%s` and `--boundary`?\n\n> +\targv_array_push(&p->log_proc.args, \"--\");\n> +\n> +\tif (start_command(&p->log_proc))\n> +\t\tdie(_(\"cannot start log\"));\n> +\n> +\tp->stream = xfdopen(p->log_proc.out, \"r\");\n> +\n> +\tp->ref_i = end_ref_i;\n> +\n> +\treturn 1;\n> +}\n\nOkay, so far, it looks like the logic to determine the tips and the\nmerge bases could easily be folded into `revision.c` guarded by a new\noption. Good.\n\n> +\n> +static const char *color_on(const char *c)\n> +{\n> +\treturn want_color(GIT_COLOR_UNKNOWN) ? c : \"\";\n> +}\n> +\n> +static const char *color_off(void)\n> +{\n> +\treturn want_color(GIT_COLOR_UNKNOWN) ? \"\\e[0m\" : \"\";\n> +}\n\nUgh. What's wrong with `GIT_COLOR_RESET`? Why hard-code an ANSI code\nhere?\n\n> +\n> +static void maybe_format_symrefs(\n> +\tstruct strbuf *sym_refs,\n> +\tstruct oidmap const *h2r,\n> +\tconst struct object_id *oid)\n> +{\n> +\tstruct hash_to_ref const *h2r_entry;\n> +\tsize_t ref_i;\n> +\n> +\th2r_entry = oidmap_get(h2r, oid);\n> +\n> +\tif (!h2r_entry)\n> +\t\treturn;\n> +\n> +\tstrbuf_addf(sym_refs, \" %s[\", color_on(\"\\e[1m\"));\n> +\n> +\tfor (ref_i = 0; ref_i < h2r_entry->nr; ref_i++) {\n> +\t\tchar *shortened_ref = shorten_unambiguous_ref(\n> +\t\t\th2r_entry->refs[ref_i]->refname, /*strict=*/1);\n> +\n> +\t\tif (ref_i)\n> +\t\t\tstrbuf_addch(sym_refs, ' ');\n> +\n> +\t\tstrbuf_addstr(sym_refs, shortened_ref);\n> +\t\tfree(shortened_ref);\n> +\t}\n> +\n> +\tstrbuf_addf(sym_refs, \"]%s\", color_off());\n> +}\n\nThis looks a lot like the `--decorate` code. I wonder whether you can\navoid duplicating that logic and use `--decorate` (or `%d`) directly.\n\nAnd if you cannot, how much effort it would be to teach the `--decorate`\nmachinery the (optional) tricks you want.\n\n> +\n> +static int process_log_line(struct log_processing *p)\n> +{\n> +\tconst char *in;\n> +\tsize_t hash_prefix_len = strlen(XL_HASH_PREFIX);\n> +\n> +\tstrbuf_reset(&p->raw_line);\n> +\tstrbuf_reset(&p->line_buf);\n> +\tstrbuf_reset(&p->line_prefix);\n> +\tstrbuf_reset(&p->sym_refs);\n> +\tstrbuf_reset(&p->tag_name);\n> +\n> +\tif (strbuf_getline_lf(&p->raw_line, p->stream) == EOF)\n> +\t\treturn 0;\n> +\n> +\tin = p->raw_line.buf;\n> +\n> +\twhile (*in) {\n> +\t\tstruct object_id oid;\n> +\t\tconst char *after_hash;\n> +\n> +\t\tif (p->line_prefix.len ||\n\nNow I am curious what that `line_prefix` field serves. It is not clear\nyo me, except that it basically prevents any parsing in\n`process_log_line()` and instead adding each input line character by\ncharacter. I also see that this `line_prefix` is reset at the beginning\nof this function and set later inside the loop. It's almost as if we\nsimply wanted to append the rest of the raw_line and `break;` at the end\nof this loop.\n\nWhich makes the whole flow a little awkward.\n\nWhy not start the loop by\n\n\tsize_t remaining = p->raw_line.len - (in - p->raw_line.buf);\n\t/* look for the commit hash */\n\tchar *hash_prefix = memmem(in, remaining, XL_HASH_PREFIX, hash_prefix_len);\n\n\tif (!hash_prefix) {\n\t\tstrbuf_add(&p->line_buf, in, remaining);\n\t\tbreak;\n\t}\n\n\t/* copy everything before the commit hash prefix */\n\tstrbuf_add(&p->line_buf, in, hash_prefix - in);\n\tin = hash_prefix + hash_prefix_len;\n\n\tif (parse_oid_hex(in, &oid, &after_hash)) {\n\t\tstrbuf_add(&p->line_buf, hash_prefix, hash_prefix_len);\n\t\tcontinue;\n\t}\n\n> +\t\t strncmp(XL_HASH_PREFIX, in, hash_prefix_len) ||\n> +\t\t parse_oid_hex(in + hash_prefix_len, &oid, &after_hash)) {\n> +\t\t\tstrbuf_addch(&p->line_buf, *in++);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tp->hash_count++;\n> +\t\tstrbuf_addf(&p->line_buf,\n> +\t\t\t \"%s %ld %s\",\n> +\t\t\t color_on(\"\\e[48;5;213m\\e[30m\"),\n> +\t\t\t p->hash_count,\n> +\t\t\t color_off());\n> +\n> +\t\tstrbuf_addf(&p->line_prefix,\n> +\t\t\t \"%s%.8s%s\",\n> +\t\t\t color_on(\"\\e[38;5;147m\"),\n> +\t\t\t in + hash_prefix_len,\n> +\t\t\t color_off());\n> +\t\tin = after_hash;\n> +\n> +\t\tstrbuf_addf(&p->tag_name, \"h/%ld\", p->hash_count);\n> +\t\tset_ref(p->ref_tr, p->tag_name.buf, &oid);\n\nIf you split out the ephemeral refs and make them an optional part of\n`git log`'s output, this should easily fall right into the `--decorate`\nmachinery's duties.\n\n> +\n> +\t\tmaybe_format_symrefs(&p->sym_refs, p->h2r, &oid);\n> +\t}\n> +\n> +\tfprintf(stdout, \"%8s %s%s\\n\",\n> +\t\tp->line_prefix.buf,\n> +\t\tp->line_buf.buf,\n> +\t\tp->sym_refs.buf);\n> +\n> +\treturn 1;\n> +}\n> +\n> +static void empty_hash_to_ref_map(struct oidmap *m)\n> +{\n> +\tstruct oidmap_iter i;\n> +\tstruct hash_to_ref *h2r;\n> +\toidmap_iter_init(m, &i);\n> +\n> +\twhile ((h2r = oidmap_iter_next(&i)) != NULL) {\n> +\t\tFREE_AND_NULL(h2r->refs);\n> +\t\th2r->alloc = 0;\n> +\t\th2r->nr = 0;\n> +\t}\n> +}\n> +\n> +static int add_ref(struct ref_array *refs, const char *name)\n> +{\n> +\tstruct object_id oid;\n> +\tsize_t ref_i;\n> +\n> +\t/* If we already have the ref, don't add it again. */\n> +\tfor (ref_i = 0; ref_i < refs->nr; ref_i++) {\n> +\t\tif (!strcmp(refs->items[ref_i]->refname, name))\n> +\t\t\treturn 0;\n> +\t}\n> +\n> +\tif (get_oid(name, &oid))\n> +\t\tdie(\"unknown object: %s\", name);\n> +\tref_array_push(refs, name, &oid);\n> +\n> +\treturn 1;\n> +}\n> +\n> +static void select_ref(\n> +\tstruct ref_selection *ref_sel,\n> +\tstruct ref_array *refs,\n> +\tsize_t ref_i)\n> +{\n> +\tALLOC_GROW_BY(ref_sel->items, ref_sel->nr, 1, ref_sel->alloc);\n> +\tref_sel->items[ref_sel->nr - 1] = refs->items[ref_i];\n> +}\n> +\n> +static void populate_branch_args(\n> +\tstruct ref_array *refs,\n> +\tstruct ref_selection *ref_sel,\n> +\tconst char **argv)\n> +{\n> +\tstruct ref_filter filter = {0};\n> +\tsize_t ref_i;\n> +\tsize_t ref_i_end;\n> +\tstruct strbuf no_xl_config_key = STRBUF_INIT;\n> +\n> +\tfilter.name_patterns = argv;\n> +\tfilter_refs(refs, &filter, FILTER_REFS_BRANCHES);\n> +\n> +\tref_i_end = refs->nr;\n> +\n> +\t/* Add upstream branches of each branch. */\n> +\tfor (ref_i = 0; ref_i < ref_i_end; ref_i++) {\n> +\t\tstruct branch *branch = branch_get(refs->items[ref_i]->refname);\n> +\t\tchar *short_name;\n> +\t\tconst char *upstream;\n> +\t\tint no_xl = 0;\n> +\n> +\t\tif (!branch) {\n> +\t\t\t/*\n> +\t\t\t * Not actually a branch, but might be HEAD. Select this\n> +\t\t\t * ref for display.\n> +\t\t\t */\n> +\t\t\tselect_ref(ref_sel, refs, ref_i);\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\t/*\n> +\t\t * Do not show the branch or its upstream if user configured\n> +\t\t * branch..no-xl = true\n> +\t\t */\n> +\t\tshort_name = shorten_unambiguous_ref(\n> +\t\t\tbranch->name, /*strict=*/1);\n> +\t\tstrbuf_reset(&no_xl_config_key);\n> +\t\tstrbuf_addf(&no_xl_config_key, \"branch.%s.no-xl\", short_name);\n> +\t\tFREE_AND_NULL(short_name);\n> +\n> +\t\tif (!git_config_get_bool(no_xl_config_key.buf, &no_xl) && no_xl)\n> +\t\t\tcontinue;\n> +\n> +\t\tselect_ref(ref_sel, refs, ref_i);\n> +\t\tupstream = branch_get_upstream(branch, NULL);\n> +\n> +\t\t/*\n> +\t\t * Add the upstream branch if it has not been added as the\n> +\t\t * upstream of some other local branch.\n> +\t\t */\n> +\t\tif (upstream && add_ref(refs, upstream))\n> +\t\t\tselect_ref(ref_sel, refs, refs->nr - 1);\n> +\t}\n> +\n> +\tstrbuf_release(&no_xl_config_key);\n> +}\n\nThis part also looks as if it would be _very_ easy to put it into\n`revision.c`, guarded by a new option.\n\nThe only thing we would need to be careful about is to clear the commit\nmarkers after figuring out the merge base(s).\n\n> +\n> +int cmd_xl(int argc, const char **argv, const char *prefx)\n> +{\n> +\tstruct oidmap hash_to_ref_map = OIDMAP_INIT;\n> +\tstruct ref_selection ref_sel = {0};\n> +\tstruct ref_array refs = {0};\n> +\tstruct strbuf ref_tr_err = STRBUF_INIT;\n> +\tstruct ref_transaction *ref_tr;\n> +\tstruct log_processing log_processing = LOG_PROCESSING_INIT;\n> +\n> +\tgit_config(git_color_config, NULL);\n> +\n> +\t/*\n> +\t * Add HEAD first. This way, if we output multiple graphs, the first\n> +\t * one will include the currently checked-out ref.\n> +\t */\n> +\tadd_ref(&refs, \"HEAD\");\n> +\n> +\tpopulate_branch_args(&refs, &ref_sel, argv + 1);\n> +\n> +\toidmap_init(&hash_to_ref_map, 16);\n> +\tpopulate_hash_to_ref_map(&hash_to_ref_map, &ref_sel);\n> +\n> +\tif (!(ref_tr = ref_transaction_begin(&ref_tr_err)))\n> +\t\tdie(\"%s\", ref_tr_err.buf);\n> +\n> +\tlog_processing.h2r = &hash_to_ref_map;\n> +\tlog_processing.ref_tr = ref_tr;\n> +\tlog_processing.refs = &ref_sel;\n> +\twhile (log_processing_start_proc(&log_processing)) {\n> +\t\twhile (process_log_line(&log_processing)) {}\n> +\t\tlog_processing_finish_proc(&log_processing);\n> +\t}\n> +\n> +\tif (ref_transaction_commit(ref_tr, &ref_tr_err))\n> +\t\tdie(\"%s\", ref_tr_err.buf);\n> +\n> +\tempty_hash_to_ref_map(&hash_to_ref_map);\n> +\toidmap_free(&hash_to_ref_map, 1);\n> +\tref_array_clear(&refs);\n> +\tref_transaction_free(ref_tr);\n> +\tstrbuf_release(&ref_tr_err);\n> +\tlog_processing_release(&log_processing);\n> +\tFREE_AND_NULL(ref_sel.items);\n> +\n> +\treturn 0;\n> +}\n> --\n> 2.19.0.605.g01d371f741-goog\n\nPhew. What a long read.\n\nShort summary of my impressions:\n\n- The _idea_ is a very useful one. Or better put: the _ideas_:\n\n\t- There is the idea of ephemeral refs, and I think it is a good\n\t one and it deserves its own patch or even its own patch\n\t series, and _definitely_ it deserves being integrated into\n\t `git log`!\n\n\t- The idea of generating the tips of the graph from the local\n\t branches that have unpushed changes, and automatically\n\t adding the merge base(s) as boundary commit(s). This deserves\n\t its own, new option in `revision.c`, I would think.\n\n- The patch mostly adds new code, in new files. This bears two problems:\n\n\t- The new code is so far away from the existing code that it is\n\t all too easy to violate the formatting conventions without\n\t even realizing, and that is quite the case.\n\n\t- Both the ephemeral refs as well as the tip/boundary selection\n\t are performed at the wrong layer.\n\n\t If they were done at the correct layer, the `git log` command\n\t would _already_ gain a lot of benefits, independent of `xl`.\n\n\t For example, a regular `git log` (with a to-be-introduced\n\t option) would generate and show the ephemeral refs. I would\n\t even go so far as to introduce a config option to make that\n\t automatic as long as outputting to a pager, that's how useful\n\t I would find this feature, personally.\n\n\t I could also imagine that introducing those features at the\n\t correct layer (`revision.c`, in both cases, I believe) would\n\t make it possible to reduce `git xl` to a simple Git alias that\n\t merely launches `git log` with a bunch o' options.\n\nThank you for starting work on this. I hope, out of purely selfish\nreasons, that you will follow through and make these two ideas a reality.\n\nCiao,\nDscho\n"},{"id":"385237","messageId":"b0169b2b-0d8a-ee27-d0f4-6c7a6df55b5d@gmail.com","threadId":"52145","inReplyTo":"nycvar.QRO.7.76.6.1910310851300.46@tvgsbejvaqbjf.bet","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2019-10-31T20:04:00Z","receivedAt":"2019-10-31T20:04:06Z","isPatch":false,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi\n\nOn 31/10/2019 08:26, Johannes Schindelin wrote:\n> Hi,\n> \n> On Wed, 30 Oct 2019, Emily Shaffer wrote:\n> \n>> On Mon, Oct 28, 2019 at 05:30:23PM -0700, Matthew DeVore wrote:\n>>> From: Matthew DeVore \n>>> [...]\n> \n> In addition, I would think that the introduction of ephemeral refs\n> should deserve its own patch. Such ephemeral refs might come in handy\n> for more things than just `xl` (or whatever better name we find).\n> \n> The design of such ephemeral refs is thoroughly interesting, too.\n\nI agree the ephemeral refs are interesting and could be really useful\n\n> One very obvious question is whether you want these refs to be\n> worktree-specific or not. I would tend to answer \"yes\" to that question.\n\nIf we go for a per-terminal namespace as you suggest below I'm not sure \nif we want/need them to be per worktree as well. I don't have a clear \nidea how I'd want to use ephemeral refs if I changed worktree but kept \nworking in the same terminal.\n\n> Further, another obvious question is what to do with those refs after a\n> while. They are _clearly_ intended to be ephemeral, i.e. they should\n> just vanish after a reasonably short time. Which raises the question:\n> what is \"reasonably short\" in this context? We would probably want to\n> come up with a good default and then offer a config setting to change\n> it.\n\nMaybe keep them around for a couple of days? Even that might be longer \nthan we need.\n\n> Another important aspect is the naming. The naming schema you chose\n> (`h/`) is short-and-sweet, and might very well be in use\n> already, for totally different purposes. It would be a really good idea\n> to open that schema to allow for avoiding clashes with already-existing\n> refs.\n> \n> A better alternative might be to choose a naming schema that cannot\n> clash with existing refs because it would not make for valid ref names.\n> I had a look at the ref name validation, and `^` might be a\n> better naming schema to begin with: `^1` is not a valid ref name, for\n> example.\n\nThat's an interesting idea, it's short and wont tread on anyone's toes.\n\n> Side note: why `h/`? I really tried to think about possible motivations\n> and came up empty.\n> \n> Another aspect that I think should be considered: why limit these\n> ephemeral refs to `git xl`? I cannot count how often I look through\n> some `git log -- `\n> to find a certain commit and then need to reference it. I usually move\n> my hand to move the mouse pointer and double click, then Shift-Insert\n> (which is awkward on this here keyboard because Insert is Fn+Delete, so\n> I cannot do that with one hand), and I usually wish for some better way.\n> \n> A better way might be to introduce an option for generating and\n> displaying such ephemeral refs, in my case it would be good to have a\n> config setting to do that automatically for every `git log` call that\n> uses the pager, i.e. is interactive.\n\nHaving them as a feature of the rev listing machinery rather than \nspecific to a particular command sounds like a good way to go.\n\n> Finally, I could imagine that in this context, we would love to have\n> refs that are purely intended for interactive use, and therefore it\n> would make sense to try to bind them to the process ID of the process\n> calling `git`, i.e. the interactive shell. That way, when I have two\n> terminal windows, they would \"own\" their separate ephemeral refs.\n\nI like that idea, though I think it should probably be based around \ngetsid() rather than getppid() (I'm not sure how that translates to windows)\n\n\nBest Wishes\n\nPhillip\n\n>>> The test cases show non-trivial output which can be used to get an idea\n>>> for what the command is good for, though it doesn't capture the\n>>> coloring.\n>>>\n>>> The primary goals of this command are:\n>>>\n>>> a) deduce what the user wants to see based on what they haven't pushed\n>>> upstream yet\n>>> b) show the active branches spatially rather than as a linear list (as\n>>> in \"git branch\")\n>>> c) allow the user to easily refer to commits that appeared in the\n>>> output\n>>>\n>>> I considered making the h/# tags stable across invocations such that a\n>>> particular hash will only be tagged with a different number if ~100\n>>> other hashes are tagged since the hash was last tagged. I didn't\n>>> actually implement it this way, instead opting for always re-numbering\n>>> the hashes on each invocation. This means the hash number is\n>>> predictable based on the position the hash appears in the output, which\n>>> is probably better that encouraging users to memorize hash numbers (or\n>>> use them in scripts!).\n>>\n>> If you're worried about folks using something like this in a script (and\n>> I would be, given that it's dynamically assigning nicknames to hashes)\n>> then you probably ought to mark it as a porcelain command in\n>> command-list.txt.\n> \n> I would like to caution against targeting scripts with this. It is too\n> easy for two concurrently running scripts to stumble over each other.\n> \n> Scripts should use safer methods that already exist, like grabbing the\n> hash while looking for a specific pattern (`sed`'s hold space comes to\n> mind).\n> \n> Ciao,\n> Dscho\n> \n"},{"id":"385273","messageId":"nycvar.QRO.7.76.6.1911011956580.46@tvgsbejvaqbjf.bet","threadId":"52145","inReplyTo":"b0169b2b-0d8a-ee27-d0f4-6c7a6df55b5d@gmail.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2019-11-01T18:58:01Z","receivedAt":"2019-11-01T18:58:26Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Phillip,\n\nOn Thu, 31 Oct 2019, Phillip Wood wrote:\n\n> On 31/10/2019 08:26, Johannes Schindelin wrote:\n>\n> > Finally, I could imagine that in this context, we would love to have\n> > refs that are purely intended for interactive use, and therefore it\n> > would make sense to try to bind them to the process ID of the\n> > process calling `git`, i.e. the interactive shell. That way, when I\n> > have two terminal windows, they would \"own\" their separate ephemeral\n> > refs.\n>\n> I like that idea, though I think it should probably be based around\n> getsid() rather than getppid() (I'm not sure how that translates to\n> windows)\n\nGood idea.\n\nOn Windows, we would probably use the `HANDLE` of the associated Win32\nConsole.\n\nCiao,\nDscho\n"},{"id":"389188","messageId":"20200103025115.GA6521@comcast.net","threadId":"52145","inReplyTo":"20191031003929.GA22855@google.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Matthew DeVore","fromEmail":"matvore@comcast.net","sentAt":"2020-01-03T02:51:15Z","receivedAt":"2020-01-03T02:59:32Z","isPatch":false,"sender":{"key":"matvore@comcast.net","avatar":"https://gravatar.com/avatar/550c64ce544f82818ad931e244dfb08bbb1febfa6d1ce3cfd65e76215ca0ac8a?d=mp&s=160"},"body":"Sorry for going dark on this topic. I'm still interested in working on this.\nI've gotten so much feedback that I fear I won't be able to respond to all of\nit in a thorough manner, but if that's the case, rest assured I have read your\nfeedback at least twice (including that from Phillip and Dscho) and will take\nit into consideration going forward.\n\nOn Wed, Oct 30, 2019 at 05:39:29PM -0700, Emily Shaffer wrote:\n> \n> Good to hear from you. One comment - the subject of your mail is \"[RFC]\"\n> but I think folks are used to receiving mails with RFC patches if the\n> subject line is formatted like it comes out of 'git format-patch' - that\n> is, [RFC PATCH].\n> \n\nThanks for the tip.\n\n> > \n> > \"git xl\" shows a graph of recent history, including all existing\n> > branches (unless flagged with a config option) and their upstream\n> > counterparts. It is named such because it is easy to type and the\n> > letter \"x\" looks like a small graph.\n> \n> For me, that's not a very compelling reason to name something, and the\n> only command with such a cryptic name in Git that I can think of is 'git\n> am'. (mv, gc, rm, and p4 are somewhat self explanatory, and everything\n> else besides 'gitk' is named with a full word.)\n\nMy thinking was that this would be a very common command, so it ought to be easy\nto type. It would also be learned pretty early. I can't blame you for disliking\ncryptic names, though. Here are some other ideas:\n\n - wip: for \"work in progress\" since it shows your repo minus upstreamed content\n - xlog: for \"x\" that looks like a graph (also, it sounds like \"extended\") and\n \"log\"\n - logx or log-x: for the same reason as above\n\nI'll be working on the \"ephemeral ref\" portion of this as a separate work item\nfor now, which doesn't require settling on a name immediately.\n\n> It looks like there's a decent amount of this commit message which\n> really ought to be a note to the reviewers instead. Everything above the\n> '---' goes into the commit message; everything below it will get\n> scrubbed when the patch is applied, so you can give more casual notes\n> there - for example this paragraph, as well as \"Omissions I might/will\n> fix\".\n> \n\nGood point, I didn't know about the \"---\" convention, so I'll keep this in mind.\n\n> \n> If you're worried about folks using something like this in a script (and\n> I would be, given that it's dynamically assigning nicknames to hashes)\n> then you probably ought to mark it as a porcelain command in\n> command-list.txt.\n> \n\nI've made a note to add this to command-list.txt.\n\nThank you,\nMatt\n"},{"id":"389198","messageId":"20200103201423.GA20975@comcast.net","threadId":"52145","inReplyTo":"nycvar.QRO.7.76.6.1910310851300.46@tvgsbejvaqbjf.bet","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Matthew DeVore","fromEmail":"matvore@comcast.net","sentAt":"2020-01-03T20:14:23Z","receivedAt":"2020-01-03T20:14:33Z","isPatch":false,"sender":{"key":"matvore@comcast.net","avatar":"https://gravatar.com/avatar/550c64ce544f82818ad931e244dfb08bbb1febfa6d1ce3cfd65e76215ca0ac8a?d=mp&s=160"},"body":"On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:\n> \n> am stands for \"apply mbox\", and I think that the only reason it is not\n> called `git apply-mbox` is that the Linux maintainer uses it a lot and\n> wanted to save on keystrokes.\n> \n> Having said that, I do agree that `xl` is not a good name for this. It\n> is neither intuitive, nor is it particularly easy to type (on a\n> US-English keyboard, the `x` and the `l` key are far apart), and to add\n\nThere is a subjective element to this, but I would consider it easy to type\nsince it is using two different hands. The property of \"keys are far apart\" is\nonly bad if it's the same or close fingers doing the typing (i.e. on qwerty\nlayout \"ve\" or \"my\")\n\nI'm not trying to justify an unpopular name, though :) There are other reasons\nto avoid \"xl\". I just found your statement surprising.\n\n> insult to injury, _any_ two-letter command is likely to shadow\n> already-existing aliases that users might have installed locally.\n> \n\n\"wip\" seems more descriptive to me, or \"logx\", as I mentioned in the reply to\nEmily.\n\n> In addition, I would think that the introduction of ephemeral refs\n> should deserve its own patch. Such ephemeral refs might come in handy\n> for more things than just `xl` (or whatever better name we find).\n> \n> The design of such ephemeral refs is thoroughly interesting, too.\n> \n> One very obvious question is whether you want these refs to be\n> worktree-specific or not. I would tend to answer \"yes\" to that question.\n\nWe could key each set of ephemeral refs off of the ttyname(3) or as you\nsuggested getsid(2). As you say, the Windows analog would be the handle of the\nWin32 console. (I'm guessing there is no concept of a terminal multiplexer\nunless you're using MinGW or WSL, in which case we can use getsid).\n\ngetsid(2) seems the least likely to overlap with previous \"keys\" so we may\nprefer that one.\n\ngetppid would not work that well if anyone ran the command (or any git command\nthat refers to the ephemeral refs) in a wrapper script (I don't mean an\nautomated script, which we definitely don't want people to try).\n\nI'm not so sure I would prefer this keying mechanism myself - I may be\ncompelled to turn it off. I sometimes have two terminals open, visible at the\nsame time, and expect them to share this kind of state. So I'm reserving\njudgment about whether it should be configurable or not. But it should probably\nbe enabled (key by session ID) by default.\n\nNow, if we key the refs off of the current session, it seems unnecessary to key\noff the worktree as well. If someone remains in the current session, but cd to\na different worktree, it would be natural for them to assume that the ephemeral\nrefs that are still visible in the terminal window would stil work.\n\n> \n> Further, another obvious question is what to do with those refs after a\n> while. They are _clearly_ intended to be ephemeral, i.e. they should\n> just vanish after a reasonably short time. Which raises the question:\n> what is \"reasonably short\" in this context? We would probably want to\n> come up with a good default and then offer a config setting to change\n> it.\n\nI would propose expiring refs as the user introduced more sessions (getsid\nvalues) without using old ones, like and LRU cache, and to limit the repository\nto holding 16 getsid keys at a time. This way, we don't have concept of a\nreal-world clock, and we let people go back to a terminal window which they\nleft open for a month and still use refs that were left there (assuming of\ncourse they haven't been using the repository heavily otherwise, and the\nterminal content is still showing those ref numbers for them to refer to).\n\nNow, if in session 42, the user generated some ephemeral refs with\n\"git log --ephemeral-refs\", these would automatically destroy any existing\nephemeral refs that were created by past invocations in session 42. I don't\nknow how important it is that we clean those up, but it seems like the right\nthing to do anyway to save disk space (at least 40 bytes per commit).\n\n> \n> Another important aspect is the naming. The naming schema you chose\n> (`h/`) is short-and-sweet, and might very well be in use\n> already, for totally different purposes. It would be a really good idea\n> to open that schema to allow for avoiding clashes with already-existing\n> refs.\n> \n> A better alternative might be to choose a naming schema that cannot\n> clash with existing refs because it would not make for valid ref names.\n> I had a look at the ref name validation, and `^` might be a\n> better naming schema to begin with: `^1` is not a valid ref name, for\n> example.\n\nI like having a new kind of syntax to make the ref names easier to type as well\nas non-conflicting with current use cases. \"^\" is hard-to-type if you're not\na good touch-typist, but I guess that's fine. If you're a good touch-typist,\n\"^\" seems a tad easier to type than \"h/\" IMO.\n\nI don't see any mention of \"%\" in \"gitrevisions(7)\" so maybe that's OK to use?\nThat is a little more of an everyday symbol than \"^\" so users are likely used to\ntyping it, and is closer to the fingers' home position. But if I remember right\nthis has special meaning in Windows shell (expand variables), so I guess it's\nnot a good idea.\n\n> \n> Side note: why `h/`? I really tried to think about possible motivations\n> and came up empty.\n> \n\nMostly because it's easy to type and didn't require exotic new syntax :) And the\n\"h\" stands for hash.\n\n> I would like to caution against targeting scripts with this. It is too\n> easy for two concurrently running scripts to stumble over each other.\n\nI think my wording before was too confusing. I totally agree we should\ndiscourage automated scripts. Convenience scripts that are meant to be used\ninteractively (e.g. glorified aliases and workflow-optimization scripts) should\nbe allowed, and I don't think we need to do anything special to make that work.\n\nThank you for the feedback!\n- Matt\n"},{"id":"389199","messageId":"xmqqk168cjn0.fsf@gitster-ct.c.googlers.com","threadId":"52145","inReplyTo":"20200103201423.GA20975@comcast.net","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-03T21:30:59Z","receivedAt":"2020-01-03T21:31:08Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matthew DeVore writes:\n\n> On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:\n>> \n>> am stands for \"apply mbox\", and I think that the only reason it is not\n>> called `git apply-mbox` is that the Linux maintainer uses it a lot and\n>> wanted to save on keystrokes.\n\nNo need to give an incorrect speculation if you do not know the\nhistory in this discussion. Back then, the command to apply mbox\ncontents existed and was called \"git applymbox\". \"am\" was invented\nas a better replacement with more rational behaviour and set of\ncommand line arguments.\n\n>> Having said that, I do agree that `xl` is not a good name for this. It\n>> is neither intuitive, nor is it particularly easy to type (on a\n>> US-English keyboard, the `x` and the `l` key are far apart), and to add\n>\n> There is a subjective element to this, but I would consider it easy to type\n> since it is using two different hands....\n\nGive descriptive name to the command, define an alias of your choice\nand use it privately. Nobody would be able to guess what \"git xl\"\nor \"git extra-long\" command would do ;-)\n"},{"id":"389222","messageId":"nycvar.QRO.7.76.6.2001042115550.46@tvgsbejvaqbjf.bet","threadId":"52145","inReplyTo":"xmqqk168cjn0.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-04T20:30:28Z","receivedAt":"2020-01-04T20:30:54Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 3 Jan 2020, Junio C Hamano wrote:\n\n> Matthew DeVore writes:\n>\n> > On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:\n> >>\n> >> am stands for \"apply mbox\", and I think that the only reason it is not\n> >> called `git apply-mbox` is that the Linux maintainer uses it a lot and\n> >> wanted to save on keystrokes.\n>\n> No need to give an incorrect speculation if you do not know the\n> history in this discussion.\n\nOh, but where would be the fun in _not_ speculating???\n\n:-)\n\n> Back then, the command to apply mbox contents existed and was called\n> \"git applymbox\". \"am\" was invented as a better replacement with more\n> rational behaviour and set of command line arguments.\n\nNow that you mention it, I vaguely remember reading about it. But even\nback then, I was not so much enthused with the idea of exporting Git\nhistory into emails and then turning those emails back into Git history\n(now with \"New And Improved!\" commit names), so I did actually not pay\nmuch attention.\n\nAs you might recall, I was also a fervent opponent of `git rebase` (which\nI think was based on `git am` from the get-go), claiming that history\nshould not be rewritten. Well, what did I know. I went on to write\n`git-edit-patch-series.sh` which you accepted into Git as `git rebase\n--interactive`, so there.\n\n> >> Having said that, I do agree that `xl` is not a good name for this.\n> >> It is neither intuitive, nor is it particularly easy to type (on a\n> >> US-English keyboard, the `x` and the `l` key are far apart), and to\n> >> add\n> >\n> > There is a subjective element to this, but I would consider it easy to\n> > type since it is using two different hands....\n>\n> Give descriptive name to the command, define an alias of your choice and\n> use it privately. Nobody would be able to guess what \"git xl\" or \"git\n> extra-long\" command would do ;-)\n\nI thought I made the point already that such short names are prone to be\nalready used by users' aliases, and that shorter command names are very\nlikely to break someone's setup.\n\nWhile I do not have any `xl` alias defined, I have 20 custom two-letter\naliases, and I would be utterly surprised if there were less than a\nthousand Git users who defined `xl` to mean something already (by now,\nthere are _a lot_ of Git users out there, and it would be foolish to\nassume that less than even the tiny fraction of a percent that translates\ninto a thousand users didn't use this alias). While one might say that\nforcing a thousand users to adjust is not a big deal, I would counter that\nwe should not, unless really necessary.\n\nAnd in this case, I deem it totally not necessary at all.\n\nBut again, I was wrong before (see e.g. the `git rebase` comment above),\nso what do I know.\n\nIn any case, as stated before, I would like to see this feature be\nimplemented as a `git log` (or even `git rev-list`) option before\nimplementing a dedicated command.\n\nIn other words, this new feature should be treated as a _mode_ rather than\na new command. The command can come later, just like `git whatchanged`\nis essentially a special-case version of `git log`.\n\nCiao,\nDscho\n"},{"id":"389225","messageId":"nycvar.QRO.7.76.6.2001042131430.46@tvgsbejvaqbjf.bet","threadId":"52145","inReplyTo":"20200103201423.GA20975@comcast.net","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-01-04T21:21:59Z","receivedAt":"2020-01-04T21:22:25Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matthew,\n\nOn Fri, 3 Jan 2020, Matthew DeVore wrote:\n\n> On Thu, Oct 31, 2019 at 09:26:48AM +0100, Johannes Schindelin wrote:\n> >\n> > am stands for \"apply mbox\", and I think that the only reason it is not\n> > called `git apply-mbox` is that the Linux maintainer uses it a lot and\n> > wanted to save on keystrokes.\n> >\n> > Having said that, I do agree that `xl` is not a good name for this. It\n> > is neither intuitive, nor is it particularly easy to type (on a\n> > US-English keyboard, the `x` and the `l` key are far apart), and to add\n>\n> There is a subjective element to this, but I would consider it easy to type\n> since it is using two different hands. The property of \"keys are far apart\" is\n> only bad if it's the same or close fingers doing the typing (i.e. on qwerty\n> layout \"ve\" or \"my\")\n\nOf course it is subjective! That's what I pointed out. And based on that\nreasoning, I think it would be a mistake to use that name: it is _waaaaay_\ntoo subjective.\n\n> I'm not trying to justify an unpopular name, though :) There are other\n> reasons to avoid \"xl\". I just found your statement surprising.\n\nI hope I got my point across. I still think that my reason to avoid `xl`\nshould have been enough, even without all those other reasons (which I\nactually not recall, this thread being so stale by now).\n\n> > insult to injury, _any_ two-letter command is likely to shadow\n> > already-existing aliases that users might have installed locally.\n>\n> \"wip\" seems more descriptive to me,\n\n\"wip\" says \"Work-In-Progress\" to me. I would strongly suspect `git wip` to\nmean something similar to `git stash`.\n\nSo no, it does not strike me as a good name for your command because it\nsuggests something _totally_ different to me, and I am not exactly what\nyou might call a Git newbie.\n\n> or \"logx\", as I mentioned in the reply to Emily.\n\nThat name does not get my support, either. My mathematician self\nassociates `logx` with the natural logarithm of `x`.\n\nI don't find this intuitive at all.\n\nMind, there are tons of unintuitive parts in Git's UI, but that should not\nencourage anyone to make the situation even worse. To the contrary, it\nshould encourage you to do better than what is there already (think \"Lake\nWobegon Strategy\").\n\n> > In addition, I would think that the introduction of ephemeral refs\n> > should deserve its own patch. Such ephemeral refs might come in handy\n> > for more things than just `xl` (or whatever better name we find).\n> >\n> > The design of such ephemeral refs is thoroughly interesting, too.\n> >\n> > One very obvious question is whether you want these refs to be\n> > worktree-specific or not. I would tend to answer \"yes\" to that\n> > question.\n>\n> We could key each set of ephemeral refs off of the ttyname(3) or as you\n> suggested getsid(2). As you say, the Windows analog would be the handle\n> of the Win32 console. (I'm guessing there is no concept of a terminal\n> multiplexer unless you're using MinGW or WSL, in which case we can use\n> getsid).\n>\n> getsid(2) seems the least likely to overlap with previous \"keys\" so we may\n> prefer that one.\n>\n> getppid would not work that well if anyone ran the command (or any git command\n> that refers to the ephemeral refs) in a wrapper script (I don't mean an\n> automated script, which we definitely don't want people to try).\n>\n> I'm not so sure I would prefer this keying mechanism myself - I may be\n> compelled to turn it off. I sometimes have two terminals open, visible at the\n> same time, and expect them to share this kind of state. So I'm reserving\n> judgment about whether it should be configurable or not. But it should probably\n> be enabled (key by session ID) by default.\n\nYou have a good point. This should be an add-on patch. If you won't have\nthe time or inclination to implement it, I will feel compelled to do it.\n\n> Now, if we key the refs off of the current session, it seems unnecessary to key\n> off the worktree as well.\n\nThat's probably beneficial: if I `cd` to a worktree, `git log --devore` a\nfew commits, then `cd` back and want to cherry-pick one of the previously\nshow commits, I definitely do not want the ephemeral revisions to be\nper-worktree.\n\n> If someone remains in the current session, but cd to a different\n> worktree, it would be natural for them to assume that the ephemeral refs\n> that are still visible in the terminal window would stil work.\n\nYes.\n\n> > Further, another obvious question is what to do with those refs after a\n> > while. They are _clearly_ intended to be ephemeral, i.e. they should\n> > just vanish after a reasonably short time. Which raises the question:\n> > what is \"reasonably short\" in this context? We would probably want to\n> > come up with a good default and then offer a config setting to change\n> > it.\n>\n> I would propose expiring refs as the user introduced more sessions (getsid\n> values) without using old ones, like and LRU cache, and to limit the repository\n> to holding 16 getsid keys at a time. This way, we don't have concept of a\n> real-world clock, and we let people go back to a terminal window which they\n> left open for a month and still use refs that were left there (assuming of\n> course they haven't been using the repository heavily otherwise, and the\n> terminal content is still showing those ref numbers for them to refer to).\n\nI don't know about you, but personally, when I find a window that had been\nopen for a gazillion days, there is a good chance that it is stale.\n\nFor example, I frequently find myself hitting the `Enter` key just to\ntrigger a re-rendering of the command prompt (which contains not only the\nbranch name, but also the information whether a rebase is in progress or\nnot) *just* because I suspect that that particular worktree is now at a\ndifferent branch.\n\nI imagine that I am not the only person with this particular issue, so no,\nI am not in favor of using an LRU. I _really_ think that we have to let\nthose ephemeral revisions expire based on age.\n\n> Now, if in session 42, the user generated some ephemeral refs with\n> \"git log --ephemeral-refs\", these would automatically destroy any existing\n> ephemeral refs that were created by past invocations in session 42. I don't\n> know how important it is that we clean those up, but it seems like the right\n> thing to do anyway to save disk space (at least 40 bytes per commit).\n\nI might be wrong, but in the non-public presentation I got the impression\nthat the use case was pretty much \"I call `git xl` and then I want to use\none of those commits in a subsequent Git command\".\n\nIn that respect, I really do not see the point of holding on to these\nephemeral revisions for even as much as 15 minutes. My suggestion to make\nthe maximal age configurable was more a conservative concern. I would be\nvery surprised if anybody wanted to use those ephemeral revisions for\nanything else than an immediate reference.\n\nBut even then, if such a use case arises, we can easily implement it then.\n_If_ it arises. Until then, I would rather avoid catering to unrealistic\n(read: unneeded) scenarios.\n\n> > Another important aspect is the naming. The naming schema you chose\n> > (`h/`) is short-and-sweet, and might very well be in use\n> > already, for totally different purposes. It would be a really good idea\n> > to open that schema to allow for avoiding clashes with already-existing\n> > refs.\n> >\n> > A better alternative might be to choose a naming schema that cannot\n> > clash with existing refs because it would not make for valid ref names.\n> > I had a look at the ref name validation, and `^` might be a\n> > better naming schema to begin with: `^1` is not a valid ref name, for\n> > example.\n>\n> I like having a new kind of syntax to make the ref names easier to type as well\n> as non-conflicting with current use cases. \"^\" is hard-to-type if you're not\n> a good touch-typist, but I guess that's fine. If you're a good touch-typist,\n> \"^\" seems a tad easier to type than \"h/\" IMO.\n>\n> I don't see any mention of \"%\" in \"gitrevisions(7)\" so maybe that's OK to use?\n> That is a little more of an everyday symbol than \"^\" so users are likely used to\n> typing it, and is closer to the fingers' home position. But if I remember right\n> this has special meaning in Windows shell (expand variables), so I guess it's\n> not a good idea.\n\nFrom the current `refs.c`:\n\n\t/*\n\t * How to handle various characters in refnames:\n\t * 0: An acceptable character for refs\n\t * 1: End-of-component\n\t * 2: ., look for a preceding . to reject .. in refs\n\t * 3: {, look for a preceding @ to reject @{ in refs\n\t * 4: A bad character: ASCII control characters, and\n\t * \":\", \"?\", \"[\", \"\\\", \"^\", \"~\", SP, or TAB\n\t * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set\n\t */\n\nThere is _no_ mention of `%`. In fact, `git update-ref refs/heads/% HEAD`\nsucceeds, while `git update-ref refs/heads/^ HEAD` fails with:\n\n\tfatal: update_ref failed for ref 'refs/heads/^': refusing to\n\tupdate ref with bad name 'refs/heads/^'\n\nAlso, I actually liked the implicit connotation of `^` being kind of an\nupward arrow, as if it implied to refer to something above.\n\nI fail to see any such connotation for the percent sign.\n\nMaybe you see something there that I missed?\n\n> > Side note: why `h/`? I really tried to think about possible motivations\n> > and came up empty.\n> >\n>\n> Mostly because it's easy to type and didn't require exotic new syntax :) And the\n> \"h\" stands for hash.\n\nAnd it totally clashes with a potential ref name:\n\n\t$ git update-ref refs/heads/h/1 HEAD\n\n\t$ git rev-parse h/1\n\t79208035afdb095548daae82679b7942c6bb9579\n\nShould we really _try_ to go out of our way to introduce ambiguities that\nhave not been there before? I would contend that we _do not_ want that.\nNot unless forced, and I really fail to see the necessity here.\n\n> > I would like to caution against targeting scripts with this. It is too\n> > easy for two concurrently running scripts to stumble over each other.\n>\n> I think my wording before was too confusing. I totally agree we should\n> discourage automated scripts. Convenience scripts that are meant to be used\n> interactively (e.g. glorified aliases and workflow-optimization scripts) should\n> be allowed, and I don't think we need to do anything special to make that work.\n\nI would really like to caution against even _suggesting_ such \"glorified\"\nusage of this feature. Scripts _can_, and therefore _should_, be more\nstringent than to rely on ephemeral revisions. I would really make it\nclear that this is _only_ intended for interactive use, by humans.\n\nIt strikes me as being similar to short revs: of course you _can_ use\nshortened object names in scripts, but why _would_ you? It only opens\nthose scripts to run into collisions with new objects whose names\nabbreviate to the same short object name. Those short names (and those\nephemeral revisions) come in handy only when there is a human who has to\ntype out these beasts. A script does not type, so they don't tire of using\nthe full names (or revision names).\n\nScripts which use those ephemeral revisions are very likely susceptible to\nproblems that non-ephemeral revisions simply do not have. So why even\nbother to suggest using ephemeral revisions for scripts? I would actually\ndo the opposite: discourage script writers from relying on _ephemeral_\nclues.\n\nCiao,\nDscho\n"},{"id":"389230","messageId":"xmqqv9pqbzk6.fsf@gitster-ct.c.googlers.com","threadId":"52145","inReplyTo":"nycvar.QRO.7.76.6.2001042115550.46@tvgsbejvaqbjf.bet","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-04T22:56:57Z","receivedAt":"2020-01-04T22:57:06Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> In any case, as stated before, I would like to see this feature be\n> implemented as a `git log` (or even `git rev-list`) option before\n> implementing a dedicated command.\n>\n> In other words, this new feature should be treated as a _mode_ rather than\n> a new command. The command can come later, just like `git whatchanged`\n> is essentially a special-case version of `git log`.\n\nYup, I agree that we may have plenty of commands that this can\nbecome a feature of, and if there is a good match, we should make it\na mode of an existing command, and \"git log\" might be a natural\nfirst candidate. If the focus is on the \"recent topics in flight\",\n\"git show-branch\" might be a good home. There may be some other\ncandidates.\n\nOn the other hand, this thing may be sufficiently different from\neverything else and deserves to be a separate command, just like\nnobody would think it is a sane design choice to try making \"git\nshortlog\" a mere mode of \"git log\".\n\nAn unrelated tangent, but I wonder if we want to start drafting the\ntransition plans to deprecate whatchanged. The command was invented\nabout two weeks before \"log\", but back then the latter did not know\nhow to drive diff-tree (iow, it was only about the commit log\nmessages), so they have both stayed to be \"useful\" for some time,\nuntil the underlying machinery for \"log\" matured sufficiently and\nmade \"whatchanged\" more or less a special case of \"log\". It is not\nhurting right now to keep it as-is and unmaintained, though.\n\n\n"},{"id":"391312","messageId":"20200207013918.GA459@comcast.net","threadId":"52145","inReplyTo":"nycvar.QRO.7.76.6.2001042131430.46@tvgsbejvaqbjf.bet","subject":"Re: [RFC] xl command for visualizing recent history","fromName":"Matthew DeVore","fromEmail":"matvore@comcast.net","sentAt":"2020-02-07T01:39:18Z","receivedAt":"2020-02-07T01:39:31Z","isPatch":false,"sender":{"key":"matvore@comcast.net","avatar":"https://gravatar.com/avatar/550c64ce544f82818ad931e244dfb08bbb1febfa6d1ce3cfd65e76215ca0ac8a?d=mp&s=160"},"body":"On Sat, Jan 04, 2020 at 10:21:59PM +0100, Johannes Schindelin wrote:\n> > There is a subjective element to this, but I would consider it easy to type\n> > since it is using two different hands. The property of \"keys are far apart\" is\n> > only bad if it's the same or close fingers doing the typing (i.e. on qwerty\n> > layout \"ve\" or \"my\")\n> \n> Of course it is subjective! That's what I pointed out. And based on that\n> reasoning, I think it would be a mistake to use that name: it is _waaaaay_\n> too subjective.\n> \n\nOK, the names I've given so far are pretty bad, and we don't have to give it a\nname anyway, since we can just make it a \"mode\" on an existing command, so\nthere's not a whole lot left to discuss.\n\nBut I'm confused about this particular part of this thread, and since this is\nrelated to naming in general, and I think about this kind of thing constantly\nfor a pet project, I'd like to get clarification: how exactly is \"xl\" hard to\ntype? So the keys are far apart on they keyboard - is that actually what makes\nit hard? I always thought using two separate hands made something easy to type.\n\n> > or \"logx\", as I mentioned in the reply to Emily.\n> \n> That name does not get my support, either. My mathematician self\n> associates `logx` with the natural logarithm of `x`.\n> \n> I don't find this intuitive at all.\n> \n> Mind, there are tons of unintuitive parts in Git's UI, but that should not\n> encourage anyone to make the situation even worse. To the contrary, it\n> should encourage you to do better than what is there already (think \"Lake\n> Wobegon Strategy\").\n> \n\nFair enough. I basically agree with all the other things you said about naming.\n\n> > I would propose expiring refs as the user introduced more sessions (getsid\n> > values) without using old ones, like and LRU cache, and to limit the repository\n> > to holding 16 getsid keys at a time. This way, we don't have concept of a\n> > real-world clock, and we let people go back to a terminal window which they\n> > left open for a month and still use refs that were left there (assuming of\n> > course they haven't been using the repository heavily otherwise, and the\n> > terminal content is still showing those ref numbers for them to refer to).\n> \n> I don't know about you, but personally, when I find a window that had been\n> open for a gazillion days, there is a good chance that it is stale.\n> \n\nYes, there is a good chance that it is stale, especially for your work\nflow and habits (I know not everyone garbage collects their terminals\npro-actively). But still, the text is there on the screen, and for some people,\nthe fact that it's on the screen is enough to consider it meaningful.\n\nThere is an obvious peril to choosing an expiration date for the refs, and that\nis that for someone somewhere, you chose an expiration date that was too soon.\nSo you solve it by extending the expiration date out a long time. Imagine we\ndetermine that expiration date that won't screw anyone over is 1 week in the\nfuture. Now you have no risk of bothering anyone. But what have you\naccomplished then? You have protected the user from referencing a ref which\nthey would not in their right mind think is valid because it is so old.\n\nSo you are better off not relying on time for expiration.\n\n> > > Another important aspect is the naming. The naming schema you chose\n> > > (`h/`) is short-and-sweet, and might very well be in use\n> > > already, for totally different purposes. It would be a really good idea\n> > > to open that schema to allow for avoiding clashes with already-existing\n> > > refs.\n> > >\n> > > A better alternative might be to choose a naming schema that cannot\n> > > clash with existing refs because it would not make for valid ref names.\n> > > I had a look at the ref name validation, and `^` might be a\n> > > better naming schema to begin with: `^1` is not a valid ref name, for\n> > > example.\n> >\n> > I like having a new kind of syntax to make the ref names easier to type as well\n> > as non-conflicting with current use cases. \"^\" is hard-to-type if you're not\n> > a good touch-typist, but I guess that's fine. If you're a good touch-typist,\n> > \"^\" seems a tad easier to type than \"h/\" IMO.\n> >\n> > I don't see any mention of \"%\" in \"gitrevisions(7)\" so maybe that's OK to use?\n> > That is a little more of an everyday symbol than \"^\" so users are likely used to\n> > typing it, and is closer to the fingers' home position. But if I remember right\n> > this has special meaning in Windows shell (expand variables), so I guess it's\n> > not a good idea.\n> \n> From the current `refs.c`:\n> \n> \t/*\n> \t * How to handle various characters in refnames:\n> \t * 0: An acceptable character for refs\n> \t * 1: End-of-component\n> \t * 2: ., look for a preceding . to reject .. in refs\n> \t * 3: {, look for a preceding @ to reject @{ in refs\n> \t * 4: A bad character: ASCII control characters, and\n> \t * \":\", \"?\", \"[\", \"\\\", \"^\", \"~\", SP, or TAB\n> \t * 5: *, reject unless REFNAME_REFSPEC_PATTERN is set\n> \t */\n> \n> There is _no_ mention of `%`. In fact, `git update-ref refs/heads/% HEAD`\n> succeeds, while `git update-ref refs/heads/^ HEAD` fails with:\n> \n> \tfatal: update_ref failed for ref 'refs/heads/^': refusing to\n> \tupdate ref with bad name 'refs/heads/^'\n> \n> Also, I actually liked the implicit connotation of `^` being kind of an\n> upward arrow, as if it implied to refer to something above.\n> \n> I fail to see any such connotation for the percent sign.\n> \n> Maybe you see something there that I missed?\n> \n> > > Side note: why `h/`? I really tried to think about possible motivations\n> > > and came up empty.\n> > >\n> >\n> > Mostly because it's easy to type and didn't require exotic new syntax :) And the\n> > \"h\" stands for hash.\n> \n> And it totally clashes with a potential ref name:\n> \n> \t$ git update-ref refs/heads/h/1 HEAD\n> \n> \t$ git rev-parse h/1\n> \t79208035afdb095548daae82679b7942c6bb9579\n> \n\nI don't see it as a huge problem if it conflicts with a potential ref name. This\nis an optional feature - no one is coerced to use it, so the name clash will not\ncreate an emergent problem. And the ref name prefix should be configurable.\n\nPunctuation tends to be harder to type than numbers, and numbers harder to type\nthan letters (I consider / about as hard to type as a bottom-row letter like\nZ). \"^\" is a pretty inconvenient location on the keyboard for something I may\nhave to type many times. And \"%\" is a little better (index finger need not move\nas much), but not a lot better. I would still prefer a non-exotic alphanumeric\nsequence for the ref prefix.\n\nNote that \"^\" will not work trivially - this is used in the `revset` command as\na prefix to refs. So you'll have to make the \"^\" contextually sensitive.\n\n> > > I would like to caution against targeting scripts with this. It is too\n> > > easy for two concurrently running scripts to stumble over each other.\n> >\n> > I think my wording before was too confusing. I totally agree we should\n> > discourage automated scripts. Convenience scripts that are meant to be used\n> > interactively (e.g. glorified aliases and workflow-optimization scripts) should\n> > be allowed, and I don't think we need to do anything special to make that work.\n> \n> I would really like to caution against even _suggesting_ such \"glorified\"\n> usage of this feature. Scripts _can_, and therefore _should_, be more\n> stringent than to rely on ephemeral revisions. I would really make it\n> clear that this is _only_ intended for interactive use, by humans.\n> \n\nI don't think you're getting my meaning when I say \"glorified alias.\" Imagine I\ndo this in my shell's rc file:\n\nalias badnamethatonlymattlikes=\"git branch -va && echo '--------' && git status --short\"\n\nThen I convert it to a script because the alias is getting too long:\n\n\t#!/bin/sh\n\t\n\tgit branch -va\n\techo '--------'\n\tgit status --short \"$@\"\n\t\n\tgit log --with-ephemeral-refs # ...\n\nThis should work.\n\nIf we use the PID to key off the ref, this obviously won't work, because the\nscript is already dead before you want to refer to the ref. getsid should work\nfine.\n\n- Matt\n"}]}