On Mon, Apr 25, 2016 at 10:33:13PM -0400, Mike Rappazzo wrote:
> I propose that it might make more sense to use something like
> `--abs-path` to indicate
> that the result should include an absolute path (or we could also just spell out
> `--absolute-path`). That way we don't have to add additional options
> for any other type
> that might want an absolute path.
>
> git rev-parse --git-dir --abs-path
> git rev-parse --git-common-dir --absolute-path
>
> I do understand that this might be more work than is necessary for the
> completion series
> here. Would it be unreasonable to suggest a partial implementation
> that, for now, only
> works with `--git-dir`?
I do like the concept of keeping "--absolute-path" orthogonal. The only
trick is that we need to either support it for all appropriate options,
or document which options it _does_ work with. Otherwise, we're going to
get bug reports when somebody tries "--absolute-path --git-common-dir".
It would be cleaner to provide a separate option to let people compose
the options, like:
git rev-parse --git-dir | git rev-parse --realpath
but that's a lot less efficient.
> > + if (gitdir) {
> > + char absolute_path[PATH_MAX];
> > + if (!realpath(gitdir, absolute_path))
> > + die_errno(_("unable to get absolute path"));
> > + puts(absolute_path);
> > + continue;
> > + }
I don't recall if this came up in earlier review, but I happened to
notice the use of realpath() here. We should be using our custom
real_path() instead. There are some platforms without realpath(), I
think, and our real_path() is not limited to the static PATH_MAX (which
is too small on some platforms).
-Peff
--
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