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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



(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 <gitster@xxxxxxxxx> wrote:
> 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.

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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]