{"thread":{"id":"40326","subject":"[PATCH] Pass graph width to pretty formatting, so '%>|' can work properly","startedAt":"2015-09-11T13:47:40Z","lastAt":"2015-09-11T20:25:30Z","messageCount":8,"participants":["Josef Kufner","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"269767","messageId":"1441979260-1494-1-git-send-email-josef@kufner.cz","threadId":"40326","inReplyTo":null,"subject":"[PATCH] Pass graph width to pretty formatting, so '%>|' can work properly","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2015-09-11T13:47:40Z","receivedAt":"2015-09-11T13:47:40Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"Pass graph width to pretty formatting function, so it can handle\n'%>|(N)' paddings correctly when --graph option is used.\n\nExample:\n  git log --all --graph --pretty='format: [%>|(20)%h] %ar%d'\n\n  All commit hashes should be aligned at 20th column from edge of the\n  terminal, not from the edge of the graph.\n\nSigned-off-by: Josef Kufner <josef@kufner.cz>\n---\n commit.h   | 1 +\n graph.c    | 7 +++++++\n graph.h    | 5 +++++\n log-tree.c | 2 ++\n pretty.c   | 1 +\n 5 files changed, 16 insertions(+)\n\ndiff --git a/commit.h b/commit.h\nindex 5d58be0..0a9a707 100644\n--- a/commit.h\n+++ b/commit.h\n@@ -160,6 +160,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 c25a09a..4802411 100644\n--- a/graph.c\n+++ b/graph.c\n@@ -671,6 +671,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@@ -68,6 +68,11 @@ 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  */\n \ndiff --git a/log-tree.c b/log-tree.c\nindex 7b1b57a..08fd5b6 100644\n--- a/log-tree.c\n+++ b/log-tree.c\n@@ -686,6 +686,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 151c2ae..f1cf9e2 100644\n--- a/pretty.c\n+++ b/pretty.c\n@@ -1297,6 +1297,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) {\n-- \n2.5.1\n"},{"id":"269783","messageId":"xmqq1te428xl.fsf@gitster.mtv.corp.google.com","threadId":"40326","inReplyTo":"1441979260-1494-1-git-send-email-josef@kufner.cz","subject":"Re: [PATCH] Pass graph width to pretty formatting, so '%>|' can work properly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-11T16:54:30Z","receivedAt":"2015-09-11T16:54:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"This \"feels\" correct ;-), but let me summon Duy who did the %><\npadding at around a5752342 (pretty: support padding placeholders, %<\n%> and %><, 2013-04-19) for an extra set of eyes.\n\nCare to add a test or two while at it?\n\nThanks.  \n"},{"id":"269785","messageId":"1441993845-25778-1-git-send-email-josef@kufner.cz","threadId":"40326","inReplyTo":"xmqq1te428xl.fsf@gitster.mtv.corp.google.com","subject":"[PATCH] Add tests for \"Pass graph width to pretty formatting, so '%>|' can work properly\"","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2015-09-11T17:50:45Z","receivedAt":"2015-09-11T17:50:45Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"---\n t/t4205-log-pretty-formats.sh | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 7398605..3358837 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -319,6 +319,18 @@ EOF\n \ttest_cmp expected actual\n '\n \n+# Note: Space between 'message' and 'two' should be in the same column 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+\tqz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >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 with no padding' '\n \tgit log --pretty=\"tformat:%>(1)%s\" >actual &&\n \tcat <<EOF >expected &&\n@@ -330,6 +342,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.5.1\n"},{"id":"269786","messageId":"55F314E4.40408@kufner.cz","threadId":"40326","inReplyTo":"xmqq1te428xl.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Pass graph width to pretty formatting, so '%>|' can work properly","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2015-09-11T17:52:36Z","receivedAt":"2015-09-11T17:52:36Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"Ok, there are the two tests (see the second e-mail). It should cover two\ncases where something may go wrong.\n\n\nJunio C Hamano wrote, on 11.9.2015 18:54:\n> This \"feels\" correct ;-), but let me summon Duy who did the %><\n> padding at around a5752342 (pretty: support padding placeholders, %<\n> %> and %><, 2013-04-19) for an extra set of eyes.\n> \n> Care to add a test or two while at it?\n> \n> Thanks.  \n> \n\n"},{"id":"269792","messageId":"CAPig+cRkSRSC0eaBencw8cFrd64sUBQE44aSJ0T2WyPg2JTFCw@mail.gmail.com","threadId":"40326","inReplyTo":"1441993845-25778-1-git-send-email-josef@kufner.cz","subject":"Re: [PATCH] Add tests for \"Pass graph width to pretty formatting, so '%>|' can work properly\"","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-09-11T19:37:53Z","receivedAt":"2015-09-11T19:37:53Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Fri, Sep 11, 2015 at 1:50 PM, Josef Kufner <josef@kufner.cz> wrote:\n> ---\n\nMissing sign-off. Or is this intended to be concatenated with the\npatch you sent earlier?\n\n> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n> index 7398605..3358837 100755\n> --- a/t/t4205-log-pretty-formats.sh\n> +++ b/t/t4205-log-pretty-formats.sh\n> @@ -319,6 +319,18 @@ EOF\n>         test_cmp expected actual\n>  '\n>\n> +# Note: Space between 'message' and 'two' should be in the same column as in previous test.\n> +test_expect_success 'right alignment formatting at the nth column with --graph. i18n.logOutputEncoding' '\n> +       git -c i18n.logOutputEncoding=$test_encoding log --graph --pretty=\"tformat:%h %>|(40)%s\" >actual &&\n> +       qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n\nYou don't seem to be taking advantage of qz_to_tab_space's\ntransliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/\nwould make the code clearer.\n\n> +* $head1                    message two\n> +* $head2                    message one\n> +* $head3                        add bar\n> +* $head4            $(commit_msg)\n> +EOF\n> +       test_cmp expected actual\n> +'\n> +\n>  test_expect_success 'right alignment formatting with no padding' '\n>         git log --pretty=\"tformat:%>(1)%s\" >actual &&\n>         cat <<EOF >expected &&\n> @@ -330,6 +342,17 @@ EOF\n>         test_cmp expected actual\n>  '\n>\n> +test_expect_success 'right alignment formatting with no padding and with --graph' '\n> +       git log --graph --pretty=\"tformat:%>(1)%s\" >actual &&\n> +       cat <<EOF >expected &&\n> +* message two\n> +* message one\n> +* add bar\n> +* $(commit_msg)\n> +EOF\n> +       test_cmp expected actual\n> +'\n"},{"id":"269793","messageId":"xmqqoah8zp36.fsf@gitster.mtv.corp.google.com","threadId":"40326","inReplyTo":"CAPig+cRkSRSC0eaBencw8cFrd64sUBQE44aSJ0T2WyPg2JTFCw@mail.gmail.com","subject":"Re: [PATCH] Add tests for \"Pass graph width to pretty formatting, so '%>|' can work properly\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-09-11T20:19:09Z","receivedAt":"2015-09-11T20:19:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Eric Sunshine <sunshine@sunshineco.com> writes:\n\n>> +       qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n>\n> You don't seem to be taking advantage of qz_to_tab_space's\n> transliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/\n> would make the code clearer.\n\nDon't cat into a pipe unless you are concatenating multiple files,\nthough ;-).\n"},{"id":"269794","messageId":"1442002952-8834-1-git-send-email-josef@kufner.cz","threadId":"40326","inReplyTo":"CAPig+cRkSRSC0eaBencw8cFrd64sUBQE44aSJ0T2WyPg2JTFCw@mail.gmail.com","subject":"[PATCH] Add tests for \"Pass graph width to pretty formatting, so '%>|' can work properly\"","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2015-09-11T20:22:32Z","receivedAt":"2015-09-11T20:22:32Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"Signed-off-by: Josef Kufner <josef@kufner.cz>\n---\n t/t4205-log-pretty-formats.sh | 23 +++++++++++++++++++++++\n 1 file changed, 23 insertions(+)\n\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 7398605..3358837 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -319,6 +319,18 @@ EOF\n \ttest_cmp expected actual\n '\n \n+# Note: Space between 'message' and 'two' should be in the same column 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+\tqz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >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 with no padding' '\n \tgit log --pretty=\"tformat:%>(1)%s\" >actual &&\n \tcat <<EOF >expected &&\n@@ -330,6 +342,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.5.1\n"},{"id":"269795","messageId":"55F338BA.6070201@kufner.cz","threadId":"40326","inReplyTo":"CAPig+cRkSRSC0eaBencw8cFrd64sUBQE44aSJ0T2WyPg2JTFCw@mail.gmail.com","subject":"Re: [PATCH] Add tests for \"Pass graph width to pretty formatting, so '%>|' can work properly\"","fromName":"Josef Kufner","fromEmail":"josef@kufner.cz","sentAt":"2015-09-11T20:25:30Z","receivedAt":"2015-09-11T20:25:30Z","isPatch":true,"sender":{"key":"josef@kufner.cz","avatar":"https://avatars.githubusercontent.com/u/16572?v=4"},"body":"Eric Sunshine wrote, on 11.9.2015 21:37:\n> On Fri, Sep 11, 2015 at 1:50 PM, Josef Kufner <josef@kufner.cz> wrote:\n>> ---\n> \n> Missing sign-off. Or is this intended to be concatenated with the\n> patch you sent earlier?\n\nJust forgot to add it. Fixed patch on the way.\n\n\n>> diff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\n>> index 7398605..3358837 100755\n>> --- a/t/t4205-log-pretty-formats.sh\n>> +++ b/t/t4205-log-pretty-formats.sh\n>> @@ -319,6 +319,18 @@ EOF\n>>         test_cmp expected actual\n>>  '\n>>\n>> +# Note: Space between 'message' and 'two' should be in the same column as in previous test.\n>> +test_expect_success 'right alignment formatting at the nth column with --graph. i18n.logOutputEncoding' '\n>> +       git -c i18n.logOutputEncoding=$test_encoding log --graph --pretty=\"tformat:%h %>|(40)%s\" >actual &&\n>> +       qz_to_tab_space <<EOF | iconv -f utf-8 -t $test_encoding >expected &&\n> \n> You don't seem to be taking advantage of qz_to_tab_space's\n> transliteration of Q to tab and Z to space, so s/qz_to_tab_space/cat/\n> would make the code clearer.\n\nI've copied another test which tests the padding without --graph and\nadded it to test the new case. I have no idea what qz_to_tab_space can\ndo. If you wish some clean up, it should be done on older tests too :)\n\n"}]}