Josef Kufner <jo...@kufner.cz> 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 <jo...@kufner.cz> > ---
[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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html