Josef Kufner <josef@xxxxxxxxx> writes:
> Pass graph width to pretty formatting, so N in '%>|(N)' includes columns
> consumed by graph rendered when --graph option is in use.
>
> Example:
> git log --all --graph --pretty='format: [%>|(20)%h] %ar%d'
>
> All commit hashes should be aligned at 20th column from edge of the
> terminal, not from the edge of the graph.
>
> Signed-off-by: Josef Kufner <josef@xxxxxxxxx>
> ---
[CC'ed Duy for ideas, as the "%>|(ALIGN)" thing is his invention]
If you imagine an entry for a commit in the "git log" output as a
rectangle that has its commit log message, "--graph" draws a bunch
of lines on the left hand side and place these rectangles on the
right of these lines. Space allocated to each of these rectangles
may and do begin at a different column. For example, here is an
output from
$ git log -12 --graph --oneline
* 7d211c9 Merge branch 'jk/graph-format-padding' into pu
|\
| * ead86db pretty: pass graph width to pretty formatting
* | 5be4874 Merge branch 'ld/p4-detached-head' into pu
|\ \
| * | 4086903 git-p4: work with a detached head
| * | 6d2be82 git-p4: add failing test for submit from detached
head
* | | 7cec6a3 Merge branch 'ls/p4-lfs' into pu
|\ \ \
| * | | 5fac7db git-p4: add Git LFS backend for large file system
| * | | 53b938f git-p4: add support for large file systems
| * | | 760e31c git-p4: return an empty list if a list config has
no values
| * | | c1e88b8 git-p4: add gitConfigInt reader
| * | | 465af7a git-p4: add optional type specifier to gitConfig
reader
* | | | 5197bd3 Merge branch 'nd/clone-linked-checkout' into pu
I can understand why you would want to include the varying width of
the river on the left hand side in the "space allocated for the
commit", and under that mental model, the patch makes sense, but at
the same time, it is also a perfectly good specification to make
"%>|(20)%h" pad "%h" to 20 columns from the left edge of the space
given to the commit.
I have a suspicion that 50% of the users would appreciate this
change, and the remainder of the users see this break their
expectation. To avoid such a regression, we may be better off if we
introduced a new alignment operator that is different from '%>|(N)'
so that this new behaviour is available to those who want to use it,
without negatively affecting existing uses.
And the use of ctx->graph_width in the hunk below would be the only
thing that needs to be changed (i.e. added to occupied only when the
new alignment operator is used).
Other than that, this round of reroll looks ready for 'next'.
Duy what do you think?
Thanks.
> diff --git a/pretty.c b/pretty.c
> index 151c2ae..f1cf9e2 100644
> --- a/pretty.c
> +++ b/pretty.c
> @@ -1297,6 +1297,7 @@ static size_t format_and_pad_commit(struct strbuf *sb, /* in UTF-8 */
> if (!start)
> start = sb->buf;
> occupied = utf8_strnwidth(start, -1, 1);
> + occupied += c->pretty_ctx->graph_width;
> padding = (-padding) - occupied;
> }
> while (1) {
--
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