Nguyễn Thái Ngọc Duy [off-list ref] writes:
The first patch was from a long time ago. The concern was it may be
breaking existing user expectation [1]. I still maintain that it's a good
thing to do and should not break anything. Hence the resubmission.
I do not think "it's a good feature to have" was a question from the
beginning. Thread [1] stopped with me saying "as long as >(N) can
be used as Duy claims as a workaround to get the original behaviour,
it is good to allow using >|(N) for this new output format; I didn't
check if >(N) does behave that way, though". What was necessary to
resurrect the patch was "Yes, >(N) can be used that way and here is
a test" or something like that.
The second patch adds negative column specifier to >|() and friends.
A positive number 'n' specifies the n-th column from the left border
of the screen, '-n' specifies the n-th column from the _right_ border.
That is quite nice enhancement.
On Thu, Jun 16, 2016 at 11:25:28AM -0700, Junio C Hamano wrote:
Nguyễn Thái Ngọc Duy [off-list ref] writes:
quoted
The first patch was from a long time ago. The concern was it may be
breaking existing user expectation [1]. I still maintain that it's a good
thing to do and should not break anything. Hence the resubmission.
I do not think "it's a good feature to have" was a question from the
beginning. Thread [1] stopped with me saying "as long as >(N) can
be used as Duy claims as a workaround to get the original behaviour,
it is good to allow using >|(N) for this new output format; I didn't
check if >(N) does behave that way, though". What was necessary to
resurrect the patch was "Yes, >(N) can be used that way and here is
a test" or something like that.
Ah ok. This command with this series (using %>(n))
~/w/git/git log --format='%>(50,trunc)%s' --graph -15 origin/pu
and the system one at version 2.7.3 (using %>|(n))
/usr/bin/git log --format='%>|(50,trunc)%s' --graph -15 origin/pu
both produce the same output like this, checked with md5sum
* Merge branch 'js/am-3-merge-recursive-direct' in..
|\
| * am: make a direct call to merge_recursive
| * merge_recursive_options: introduce the "gently" ..
* | Merge branch 'tb/complete-status' into pu
|\ \
| * | completion: add git status
| * | completion: add __git_get_option_value helper
| * | completion: factor out untracked file modes into..
* | | Merge branch 'mj/log-show-signature-conf' into pu
|\ \ \
| * | | log: "--no-show-signature" commmand-line option
| * | | log: add "log.showsignature" configuration varia..
* | | | Merge branch 'nd/worktree-lock' into pu
|\ \ \ \
| * | | | worktree.c: find_worktree() search by path suffix
| * | | | worktree: add "unlock" command
| * | | | worktree: add "lock" command
| * | | | worktree.c: add is_worktree_locked()
The output looks weird though, which makes me a bit hesitate to add it
in the test suite...
--
Duy
Junio C Hamano wrote, on 16.6.2016 20:25:
"as long as >(N) can be used as Duy claims as a workaround to
get the original behaviour, it is good to allow using >|(N)
for this new output format; I didn't check if >(N) does behave
that way, though"
Yes, it can be used in such way.
Example:
$ git log --format='%<(10)%t%<(20)%h.' -n 1
004419a 9ca73ba .
$ git log --format='%<(10)%t%<|(30)%h.' -n 1
004419a 9ca73ba .
Notice that dot is at exactly same column, because
10 + 20 == 30.
The difference is when using --graph, because width of the graph is not
known in advance. After the patch the graph width is included in %<(N).
Original behavior is to not count with the graph width.