git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / Atom feed
From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>,
	git@vger.kernel.org,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] rev-parse: add option for absolute or relative path formatting
Date: Thu, 10 Sep 2020 17:19:35 +0200
Message-ID: <20200910151935.GA5265@szeder.dev> (raw)
In-Reply-To: <20200909222333.GH241078@camp.crustytoothpaste.net>

On Wed, Sep 09, 2020 at 10:23:33PM +0000, brian m. carlson wrote:
> On 2020-09-09 at 14:51:14, SZEDER Gábor wrote:
> > On Tue, Sep 08, 2020 at 06:50:17PM +0000, brian m. carlson wrote:
> > > diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> > > index 19b12b6d43..6b95292559 100644
> > > --- a/Documentation/git-rev-parse.txt
> > > +++ b/Documentation/git-rev-parse.txt
> > > @@ -208,6 +208,15 @@ Options for Files
> > >  	Only the names of the variables are listed, not their value,
> > >  	even if they are set.
> > >  
> > > +--path-format=(absolute|relative)::
> > > +	Controls the behavior of certain other options following it on the command
> > > +	line.
> > 
> > Does it affect only one subsequent such option on the command line, or
> > all such options?  IOW, while standing in the top directory of the
> > worktree, would the following command
> > 
> >   git rev-parse --path-format=absolute --git-dir --git-path foo --show-toplevel
> > 
> > print three absolute paths, or one absolute paths and two relative
> > paths?
> > 
> > The wording here is not clear on this, the commit message doesn't
> > mention it, and the tests added in this patch only check one such
> > option, but looking at the code and doing some testing of my own I
> > found that it affects all subsequent such options.
> 
> It affects all subsequent options.  Moreover, I believe it's possible to
> switch in the middle of the command line if you want some things
> relative and some absolute.  That seemed to be both the easiest solution
> and the most flexible, so I went with it.  I'll add more tests for this
> case and improve the commit message.

Well, on first sight it seems nice that scripts have to specify
'--path-format=absolute' only once when querying multiple paths at
once, though on second thought no prudent script would query multiple
paths at once, because they might contain newlines.

> >   $ ./git -C Documentation/ rev-parse --path-format=relative --git-dir --show-toplevel
> >   /home/szeder/src/git/.git
> >   /home/szeder/src/git

> >   $ ./git rev-parse --path-format=absolute --git-path foo/bar
> >   fatal: Invalid path '/home/szeder/src/git/.git/foo': No such file or directory
> 
> That's going to be a little tricky to fix, but I'll look into it.

Yeah, I think I found trouble that way, too.  I wonder whether
'--path-format=relative' is really worth having, though.  Clearly
there's a need for absolute paths, because getting relative paths
causes difficulties for some scripts; I described one such use case in
a2f5a87626 (rev-parse: add '--absolute-git-dir' option, 2017-02-03),
and you just ran into another with Git LFS.  However, is there really
a use case that requires relative paths, because an absolute path
would cause similar difficulties?  I couldn't come with any.

So perhaps it would be sufficient to introduce only
'--path-format=absolute' (or equivalent) for now, and add a relative
path variant only when there will be an actual compelling reason to do
so.  And that would save you from the pain of addressing these bugs
shown above.


  reply	other threads:[~2020-09-10 19:41 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-08 18:50 brian m. carlson
2020-09-09  3:23 ` Johannes Schindelin
2020-09-09 22:31   ` brian m. carlson
2020-09-09 14:51 ` SZEDER Gábor
2020-09-09 22:23   ` brian m. carlson
2020-09-10 15:19     ` SZEDER Gábor [this message]
2020-09-11  0:03       ` brian m. carlson
2020-11-04 22:16 ` Jonathan Nieder
2020-11-05  3:11   ` brian m. carlson
2020-11-06  0:51     ` Jonathan Nieder
2020-11-06  1:57       ` brian m. carlson

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=20200910151935.GA5265@szeder.dev \
    --to=szeder.dev@gmail.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=sandals@crustytoothpaste.net \
    /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

git@vger.kernel.org list mirror (unofficial, one of many)

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V1 git git/ https://public-inbox.org/git \
		git@vger.kernel.org
	public-inbox-index git

Example config snippet for mirrors.
Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.io/gmane.comp.version-control.git
 note: .onion URLs require Tor: https://www.torproject.org/

code repositories for the project(s) associated with this inbox:

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

AGPL code for this site: git clone https://public-inbox.org/public-inbox.git