Re: [PATCH 1/3] pretty: Pass graph width to pretty formatting for use in '%>|(N)'

(I'm joining the newer thread [1] back to this one, thanks for
reminding me about this)

[1] http://thread.gmane.org/gmane.comp.version-control.git/282771

On Tue, Sep 22, 2015 at 2:40 AM, Junio C Hamano [off-list ref] wrote:

Josef Kufner [off-list ref] writes:

quoted

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 <redacted>
---

[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.

I tend to agree with Josef. >|(N) is about columns relative to the
left edge of the screen. You can already use >(N) to be relative to
the left edge of the space given to the commit.
-- 
Duy