{"thread":{"id":"42631","subject":"[PATCH 0/2] Log pretty format alignment improvements","startedAt":"2016-06-16T13:19:05Z","lastAt":"2016-06-17T17:31:41Z","messageCount":7,"participants":["Nguyễn Thái Ngọc Duy","Junio C Hamano","Duy Nguyen","Josef Kufner"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"289363","messageId":"20160616131838.14440-1-pclouds@gmail.com","threadId":"42631","inReplyTo":null,"subject":"[PATCH 0/2] Log pretty format alignment improvements","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-16T13:18:36Z","receivedAt":"2016-06-16T13:19:05Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The first patch was from a long time ago. The concern was it may be\nbreaking existing user expectation [1]. I still maintain that it's a good\nthing to do and should not break anything. Hence the resubmission.\n\nThe second patch adds negative column specifier to >|() and friends.\nA positive number 'n' specifies the n-th column from the left border\nof the screen, '-n' specifies the n-th column from the _right_ border.\n\nI have been happy with 2/2 improving my \"git l1\" (fancy --oneline)\nprintout for many months. It's definitely a good thing to do in my\nopinion.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/277710/focus=278326\n\nJosef Kufner (1):\n  pretty: pass graph width to pretty formatting for use in '%>|(N)'\n\nNguyễn Thái Ngọc Duy (1):\n  pretty.c: support <direction>|(<negative number>) forms\n\n commit.h                      |  1 +\n graph.c                       |  7 ++++++\n graph.h                       |  5 ++++\n log-tree.c                    |  2 ++\n pretty.c                      |  9 ++++++-\n t/t4205-log-pretty-formats.sh | 57 +++++++++++++++++++++++++++++++++++++++++++\n 6 files changed, 80 insertions(+), 1 deletion(-)\n\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"289364","messageId":"20160616131838.14440-2-pclouds@gmail.com","threadId":"42631","inReplyTo":"20160616131838.14440-1-pclouds@gmail.com","subject":"[PATCH 1/2] pretty: pass graph width to pretty formatting for use in '%>|(N)'","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-16T13:18:37Z","receivedAt":"2016-06-16T13:19:07Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: Josef Kufner <josef@kufner.cz>\n\nPass graph width to pretty formatting, to make N in '%>|(N)'\ninclude columns consumed by graph rendered when --graph option\nis in use.\n\nFor example, in the output of\n\n  git log --all --graph --pretty='format: [%>|(20)%h] %ar%d'\n\nthis change will make all commit hashes align at 20th column from\nthe edge of the terminal, not from the edge of the graph.\n\nSigned-off-by: Josef Kufner <josef@kufner.cz>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n commit.h                      |  1 +\n graph.c                       |  7 +++++++\n graph.h                       |  5 +++++\n log-tree.c                    |  2 ++\n pretty.c                      |  1 +\n t/t4205-log-pretty-formats.sh | 24 ++++++++++++++++++++++++\n 6 files changed, 40 insertions(+)\n\ndiff --git a/commit.h b/commit.h\nindex b06db4d..0c923f0 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -161,6 +161,7 @@ struct pretty_print_context {\n \t * should not be counted on by callers.\n \t */\n \tstruct string_list in_body_headers;\n+\tint graph_width;\n };\n \n struct userformat_want {\ndiff --git a/graph.c b/graph.c\nindex 1350bdd..ad766fa 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -669,6 +669,13 @@ static void graph_output_padding_line(struct git_graph *graph,\n \tgraph_pad_horizontally(graph, sb, graph->num_new_columns * 2);\n }\n \n+\n+int graph_width(struct git_graph *graph)\n+{\n+\treturn graph->width;\n+}\n+\n+\n static void graph_output_skip_line(struct git_graph *graph, struct strbuf *sb)\n {\n \t/*\ndiff --git a/graph.h b/graph.h\nindex 0be62bd..3f48c19 100644\n--- a/graph.h\n+++ b/graph.h\n@@ -67,6 +67,11 @@ int graph_is_commit_finished(struct git_graph const *graph);\n int graph_next_line(struct git_graph *graph, struct strbuf *sb);\n \n \n+/*\n+ * Return current width of the graph in on-screen characters.\n+ */\n+int graph_width(struct git_graph *graph);\n+\n /*\n  * graph_show_*: helper functions for printing to stdout\n  */\ndiff --git a/log-tree.c b/log-tree.c\nindex 78a5381..8d39315 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -687,6 +687,8 @@ void show_log(struct rev_info *opt)\n \tctx.output_encoding = get_log_output_encoding();\n \tif (opt->from_ident.mail_begin && opt->from_ident.name_begin)\n \t\tctx.from_ident = &opt->from_ident;\n+\tif (opt->graph)\n+\t\tctx.graph_width = graph_width(opt->graph);\n \tpretty_print_commit(&ctx, commit, &msgbuf);\n \n \tif (opt->add_signoff)\ndiff --git a/pretty.c b/pretty.c\nindex 87c4497..4f33b09 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1299,6 +1299,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */\n \t\tif (!start)\n \t\t\tstart = sb->buf;\n \t\toccupied = utf8_strnwidth(start, -1, 1);\n+\t\toccupied += c->pretty_ctx->graph_width;\n \t\tpadding = (-padding) - occupied;\n \t}\n \twhile (1) {\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 7398605..d75cdbb 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -319,6 +319,19 @@ EOF\n \ttest_cmp expected actual\n '\n \n+# Note: Space between 'message' and 'two' should be in the same column\n+# as in previous test.\n+test_expect_success 'right alignment formatting at the nth column with --graph. i18n.logOutputEncoding' '\n+\tgit -c i18n.logOutputEncoding=$test_encoding log --graph --pretty=\"tformat:%h %>|(40)%s\" >actual &&\n+\ticonv -f utf-8 -t $test_encoding >expected <<EOF&&\n+* $head1                    message two\n+* $head2                    message one\n+* $head3                        add bar\n+* $head4            $(commit_msg)\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'right alignment formatting with no padding' '\n \tgit log --pretty=\"tformat:%>(1)%s\" >actual &&\n \tcat <<EOF >expected &&\n@@ -330,6 +343,17 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'right alignment formatting with no padding and with --graph' '\n+\tgit log --graph --pretty=\"tformat:%>(1)%s\" >actual &&\n+\tcat <<EOF >expected &&\n+* message two\n+* message one\n+* add bar\n+* $(commit_msg)\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'right alignment formatting with no padding. i18n.logOutputEncoding' '\n \tgit -c i18n.logOutputEncoding=$test_encoding log --pretty=\"tformat:%>(1)%s\" >actual &&\n \tcat <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"289365","messageId":"20160616131838.14440-3-pclouds@gmail.com","threadId":"42631","inReplyTo":"20160616131838.14440-1-pclouds@gmail.com","subject":"[PATCH 2/2] pretty.c: support <direction>|(<negative number>) forms","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-16T13:18:38Z","receivedAt":"2016-06-16T13:19:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"%>|(num), %><|(num) and %<|(num), where num is a positive number, sets a\nfixed column from the screen's left border. There is no way for us to\nspecifiy a column relative to the right border, which is useful when you\nwant to make use of all terminal space (on big screens). Use negative\nnum for that. Inspired by Go's array syntax (*).\n\n(*) I know Python has this first (or before Go, at least) but the idea\ndidn't occur to me until I learned Go.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n pretty.c                      |  8 +++++++-\n t/t4205-log-pretty-formats.sh | 33 +++++++++++++++++++++++++++++++++\n 2 files changed, 40 insertions(+), 1 deletion(-)\n\ndiff --git a/pretty.c b/pretty.c\nindex 4f33b09..1c67b23 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1022,9 +1022,15 @@ static size_t parse_padding_placeholder(struct strbuf *sb,\n \t\tint width;\n \t\tif (!end || end == start)\n \t\t\treturn 0;\n-\t\twidth = strtoul(start, &next, 10);\n+\t\twidth = strtol(start, &next, 10);\n \t\tif (next == start || width == 0)\n \t\t\treturn 0;\n+\t\tif (width < 0) {\n+\t\t\tif (to_column)\n+\t\t\t\twidth += term_columns();\n+\t\t\tif (width < 0)\n+\t\t\t\treturn 0;\n+\t\t}\n \t\tc->padding = to_column ? -width : width;\n \t\tc->flush_type = flush_type;\n \ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex d75cdbb..d9f6242 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -176,6 +176,17 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'left alignment formatting at the nth column' '\n+\tCOLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n+\tqz_to_tab_space <<EOF >expected &&\n+$head1 message two                    Z\n+$head2 message one                    Z\n+$head3 add bar                        Z\n+$head4 $(commit_msg)            Z\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'left alignment formatting at the nth column. i18n.logOutputEncoding' '\n \tgit -c i18n.logOutputEncoding=$test_encoding log --pretty=\"tformat:%h %<|(40)%s\" >actual &&\n \tqz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n@@ -308,6 +319,17 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'right alignment formatting at the nth column' '\n+\tCOLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n+\tqz_to_tab_space <<EOF >expected &&\n+$head1                      message two\n+$head2                      message one\n+$head3                          add bar\n+$head4              $(commit_msg)\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'right alignment formatting at the nth column. i18n.logOutputEncoding' '\n \tgit -c i18n.logOutputEncoding=$test_encoding log --pretty=\"tformat:%h %>|(40)%s\" >actual &&\n \tqz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n@@ -397,6 +419,17 @@ EOF\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'center alignment formatting at the nth column' '\n+\tCOLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n+\tqz_to_tab_space <<EOF >expected &&\n+$head1           message two          Z\n+$head2           message one          Z\n+$head3             add bar            Z\n+$head4       $(commit_msg)      Z\n+EOF\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'center alignment formatting at the nth column. i18n.logOutputEncoding' '\n \tgit -c i18n.logOutputEncoding=$test_encoding log --pretty=\"tformat:%h %><|(40)%s\" >actual &&\n \tqz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n-- \n2.8.2.524.g6ff3d78\n\n"},{"id":"289380","messageId":"xmqqtwgt3s9z.fsf@gitster.mtv.corp.google.com","threadId":"42631","inReplyTo":"20160616131838.14440-1-pclouds@gmail.com","subject":"Re: [PATCH 0/2] Log pretty format alignment improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-16T18:25:28Z","receivedAt":"2016-06-16T18:25:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> The first patch was from a long time ago. The concern was it may be\n> breaking existing user expectation [1]. I still maintain that it's a good\n> thing to do and should not break anything. Hence the resubmission.\n\nI do not think \"it's a good feature to have\" was a question from the\nbeginning.  Thread [1] stopped with me saying \"as long as >(N) can\nbe used as Duy claims as a workaround to get the original behaviour,\nit is good to allow using >|(N) for this new output format; I didn't\ncheck if >(N) does behave that way, though\".  What was necessary to\nresurrect the patch was \"Yes, >(N) can be used that way and here is\na test\" or something like that.\n\n> The second patch adds negative column specifier to >|() and friends.\n> A positive number 'n' specifies the n-th column from the left border\n> of the screen, '-n' specifies the n-th column from the _right_ border.\n\nThat is quite nice enhancement.\n"},{"id":"289434","messageId":"20160617112707.GA29739@ash","threadId":"42631","inReplyTo":"xmqqtwgt3s9z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Log pretty format alignment improvements","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-06-17T11:27:07Z","receivedAt":"2016-06-17T11:27:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jun 16, 2016 at 11:25:28AM -0700, Junio C Hamano wrote:\n> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n> \n> > The first patch was from a long time ago. The concern was it may be\n> > breaking existing user expectation [1]. I still maintain that it's a good\n> > thing to do and should not break anything. Hence the resubmission.\n> \n> I do not think \"it's a good feature to have\" was a question from the\n> beginning.  Thread [1] stopped with me saying \"as long as >(N) can\n> be used as Duy claims as a workaround to get the original behaviour,\n> it is good to allow using >|(N) for this new output format; I didn't\n> check if >(N) does behave that way, though\".  What was necessary to\n> resurrect the patch was \"Yes, >(N) can be used that way and here is\n> a test\" or something like that.\n\nAh ok. This command with this series (using %>(n))\n\n~/w/git/git log --format='%>(50,trunc)%s' --graph -15 origin/pu\n\nand the system one at version 2.7.3 (using %>|(n))\n\n/usr/bin/git log --format='%>|(50,trunc)%s' --graph -15 origin/pu\n\nboth produce the same output like this, checked with md5sum\n\n*   Merge branch 'js/am-3-merge-recursive-direct' in..\n|\\  \n| *          am: make a direct call to merge_recursive\n| * merge_recursive_options: introduce the \"gently\" ..\n* |            Merge branch 'tb/complete-status' into pu\n|\\ \\  \n| * |                         completion: add git status\n| * |      completion: add __git_get_option_value helper\n| * | completion: factor out untracked file modes into..\n* | |    Merge branch 'mj/log-show-signature-conf' into pu\n|\\ \\ \\  \n| * | |    log: \"--no-show-signature\" commmand-line option\n| * | | log: add \"log.showsignature\" configuration varia..\n* | | |              Merge branch 'nd/worktree-lock' into pu\n|\\ \\ \\ \\  \n| * | | |  worktree.c: find_worktree() search by path suffix\n| * | | |                     worktree: add \"unlock\" command\n| * | | |                       worktree: add \"lock\" command\n| * | | |               worktree.c: add is_worktree_locked()\n\nThe output looks weird though, which makes me a bit hesitate to add it\nin the test suite...\n--\nDuy\n"},{"id":"289457","messageId":"xmqqwpln22cc.fsf@gitster.mtv.corp.google.com","threadId":"42631","inReplyTo":"20160617112707.GA29739@ash","subject":"Re: [PATCH 0/2] Log pretty format alignment improvements","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-17T16:43:15Z","receivedAt":"2016-06-17T16:43:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Duy Nguyen <pclouds@gmail.com> writes:\n\n> Ah ok. This command with this series (using %>(n))\n>\n> ~/w/git/git log --format='%>(50,trunc)%s' --graph -15 origin/pu\n>\n> and the system one at version 2.7.3 (using %>|(n))\n>\n> /usr/bin/git log --format='%>|(50,trunc)%s' --graph -15 origin/pu\n>\n> both produce the same output like this, checked with md5sum\n>\n> *   Merge branch 'js/am-3-merge-recursive-direct' in..\n> |\\  \n> | *          am: make a direct call to merge_recursive\n> | * merge_recursive_options: introduce the \"gently\" ..\n> * |            Merge branch 'tb/complete-status' into pu\n> |\\ \\  \n> | * |                         completion: add git status\n> | * |      completion: add __git_get_option_value helper\n> | * | completion: factor out untracked file modes into..\n> * | |    Merge branch 'mj/log-show-signature-conf' into pu\n> |\\ \\ \\  \n> | * | |    log: \"--no-show-signature\" commmand-line option\n> | * | | log: add \"log.showsignature\" configuration varia..\n> * | | |              Merge branch 'nd/worktree-lock' into pu\n> |\\ \\ \\ \\  \n> | * | | |  worktree.c: find_worktree() search by path suffix\n> | * | | |                     worktree: add \"unlock\" command\n> | * | | |                       worktree: add \"lock\" command\n> | * | | |               worktree.c: add is_worktree_locked()\n>\n> The output looks weird though, which makes me a bit hesitate to add it\n> in the test suite...\n\nIt may become easier to see if you used %<(N) vs %<|(N) instead?\n"},{"id":"289461","messageId":"dc9501ec-7684-ac35-7bac-f88fab707f18@kufner.cz","threadId":"42631","inReplyTo":"xmqqtwgt3s9z.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH 0/2] Log pretty format alignment improvements","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2016-06-17T17:24:48Z","receivedAt":"2016-06-17T17:31:41Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"Junio C Hamano wrote, on 16.6.2016 20:25:\n> \"as long as >(N) can be used as Duy claims as a workaround to \n> get the original behaviour, it is good to allow using >|(N)\n> for this new output format; I didn't check if >(N) does behave\n> that way, though\"\n\nYes, it can be used in such way.\n\nExample:\n\n$ git log --format='%<(10)%t%<(20)%h.' -n 1\n004419a   9ca73ba             .\n\n$ git log --format='%<(10)%t%<|(30)%h.' -n 1\n004419a   9ca73ba             .\n\nNotice that dot is at exactly same column, because\n10 + 20 == 30.\n\nThe difference is when using --graph, because width of the graph is not\nknown in advance. After the patch the graph width is included in %<(N).\nOriginal behavior is to not count with the graph width.\n\n"}]}