git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Michael Vogt <mvo@ubuntu.com>
Cc: git@vger.kernel.org, sbeller@google.com, avarab@gmail.com
Subject: Re: [PATCH] show: add --follow-symlinks option for <rev>:<path>
Date: Tue, 17 Apr 2018 08:05:16 +0900	[thread overview]
Message-ID: <xmqqin8qwy4z.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180416093625.15752-2-mvo@ubuntu.com> (Michael Vogt's message of "Mon, 16 Apr 2018 11:36:25 +0200")

Michael Vogt <mvo@ubuntu.com> writes:

> Add a --follow-symlinks option that'll resolve symlinks to their
> targets when the target is of the form <rev>:<path>.

This not only affects "show" but all in the "log" family of
commands, because the change is made to revision.[ch] that is shared
by them.  I doubt that is desirable.

I offhand do not think of any command other than "show", to which
this feature makes any sense [*1*].  And I certainly do not mind if
the feature is limited to "show" and nothing else for that reason.

But I do mind the implementation that seeps through to other
commands (because "log_init_finish()" is shared with commands in the
family other than "show", and because "struct rev_info" is shared
across all the commands in the "log" famil) and not limited to
"show", which is a sign of typical end-user confusion waiting to
happen.

Thanks.


[Footnote]

For example, "git log" is affected by this patch but I am not sure
what it even means that we can ask this question:

	$ git log -p --follow-symlinks -- RelNotes

Can we see how each update to Documentation/RelNotes/2.17.0.txt as
well as change of RelNotes symlink from 2.17.0.txt to 2.18.0.txt in
the patch form?  If that were the case, it might make some sense to
allow the feature to be triggered by "git log" like your change to
builtin/log.c did, but I somehow do not think that is what your
patch does.


  reply	other threads:[~2018-04-16 23:05 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-09  9:00 [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt
2018-04-09  9:28 ` Ævar Arnfjörð Bjarmason
2018-04-13  9:43   ` Michael Vogt
2018-04-13 17:28     ` Stefan Beller
2018-04-13 17:48       ` Michael Vogt
2018-04-13 19:33         ` Ævar Arnfjörð Bjarmason
2018-04-13 19:48           ` [PATCH] support: git show --follow-symlinks HEAD:symlink Michael Vogt
2018-04-13 21:03             ` Ævar Arnfjörð Bjarmason
2018-04-16  9:36               ` [PATCH v2] show: add --follow-symlinks option for <rev>:<path> Michael Vogt
2018-04-16  9:36                 ` [PATCH] " Michael Vogt
2018-04-16 23:05                   ` Junio C Hamano [this message]
2018-04-13 19:55           ` [RFC PATCH] Add "git show --follow-symlinks HEAD:symlink" Michael Vogt

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqin8qwy4z.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mvo@ubuntu.com \
    --cc=sbeller@google.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).