All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jacob Keller <jacob.keller@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v5 2/3] diff: add --diff-line-prefix option for passing in a prefix
Date: Sun, 14 Aug 2016 14:21:50 -0700	[thread overview]
Message-ID: <xmqqa8gfkpw1.fsf@gitster.mtv.corp.google.com> (raw)
In-Reply-To: <CA+P7+xp_sPk6P1qyyDfOgpkXU1GxWPivfSzvveS4PAvGb-=ggQ@mail.gmail.com> (Jacob Keller's message of "Fri, 12 Aug 2016 15:59:17 -0700")

Jacob Keller <jacob.keller@gmail.com> writes:

> On Fri, Aug 12, 2016 at 2:58 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> On Fri, Aug 12, 2016 at 2:43 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> Ok so the big problem here is that unlike with diff which has support
>>> for line-prefixes already due to needing it for graph mode, I can't
>>> figure out where to get the prefix added for log output.
>>
>> Doesn't the --graph mode show the graph lines already? I think most
>> if not all of that processing happens inside log-tree.c::show_log().
>
> Yes but the problem is how to get this additional prefix to be displayed for
>
> (a) non graph mode
> (b) for graph mode but display the prefix before the graph prefix
>
> It doesn't seem straight forward, and I haven't figured out how the
> graph code already works.

Because you only need "diff --line-prefix" to work while leaving
"log --line-prefix" broken for the purpose of your immediate purpose
to update "diff/log [--graph and other options] --submodule=<type>"
to show "diff A B" in the submodule, I think it is OK to leave it
broken, as long as it is clearly documented that "--line-prefix"
should also apply to the log message part but the current code
doesn't.  Eventually, when somebody wants to add a new <type> that
runs "log -p A..B" instead of "diff A B" in submodules and wants to
make it interact well with "log --graph" at the superproject level,
they need to fix the breakage you leave behind.  That would be much
better than "because I cannot figure out how to prefix on the log
part, I'll name this --diff-line-prefix", which will force us to
support that half-baked option forever while having to add a proper
"--line-prefix" eventually anyway.

  parent reply	other threads:[~2016-08-14 21:21 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-11 22:59 [PATCH v5 1/3] diff.c: remove output_prefix_length field Jacob Keller
2016-08-11 22:59 ` [PATCH v5 2/3] diff: add --diff-line-prefix option for passing in a prefix Jacob Keller
2016-08-12 21:15   ` Junio C Hamano
2016-08-12 21:21     ` Jacob Keller
2016-08-12 21:43       ` Jacob Keller
     [not found]         ` <CAPc5daVmyx+EX8H0yETfO6Vv+A7DqBM5bsqrnJdYzbEhVnA1wQ@mail.gmail.com>
     [not found]           ` <CA+P7+xp_sPk6P1qyyDfOgpkXU1GxWPivfSzvveS4PAvGb-=ggQ@mail.gmail.com>
2016-08-14 21:21             ` Junio C Hamano [this message]
2016-08-15  5:32               ` Jacob Keller
2016-08-15 15:37                 ` Junio C Hamano
2016-08-15 23:09                   ` Jacob Keller
2016-08-11 22:59 ` [PATCH v5 3/3] diff: add SUBMODULE_DIFF format to display submodule diff Jacob Keller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqa8gfkpw1.fsf@gitster.mtv.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jacob.keller@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.