2012/3/20 Junio C Hamano <gitster@xxxxxxxxx>: > Regarding the log message: > > - Please start it with a problem description. Describe both what the > current code shows, and why you think it is wrong or suboptimal. > I.e. the observation of the problem in your second paragraph comes at > the beginning > > - Our log message usually gives an order to the codebase or to the person > who is applying the patch in order to address the problem you described > in the earlier part of the log message, instead of tells a story of > what happened in the past. > > E.g. > > The recent change to compute the width of diff --stat based on the > terminal width did not take the width needed to show the --graph > output into account, and makes lines in "log --graph --stat" too long. > > Adjust stat width calculation to take the width of graph prefix into > account. ... Thanks for letting me know. Patch v2 has updated log messages. Let me know whether they meet the conventions. > I think the caller should be taught to pass the exact width it carves out > of the available width for use by the ancestry graph output, and if we are > to do so, adding "int output_prefix_len" field (which usually is 0) to > diff_options, and seting it in graph.c::diff_output_prefix_callback() (at > that point, graph->width has the number you want, I think), may be the way > to go. Added outout_prefix_length to struct diff_options in patch v2. >> diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh >> index 328aa8f..84dd8bb 100755 >> --- a/t/t4052-stat-output.sh >> +++ b/t/t4052-stat-output.sh >> @@ -162,7 +162,7 @@ test_expect_success 'preparation for long filename tests' ' >> ' >> >> cat >expect <<'EOF' >> - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++ >> + ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 ++++++++++++++++++ >> EOF > > Isn't it a sign that the change is doing a lot more than justified that it > has to change the test vector for cases where --graph is *NOT* involved at > all? It is, and I didn't make that clear in the log message. In patch v2, the log message describes what has changed to in the calculation to cause this. Thanks! Lucian -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html