Re: [PATCH] ref-filter: don't look for objects when outside of a repository

[Jeff King <peff@peff.net>]

On Mon, Sep 24, 2018 at 11:20:34PM +0200, SZEDER Gábor wrote:

> > Would we perhaps want to give the user a hint that the object is not
> > really missing, but rather that we're not in a repository? E.g.,
> > something like:
> > 
> >   if (!have_git_dir())
> > 	return strbuf_addf_ret(err, -1, "format specifier requires a repository");
> >   if (oid_object_info_extended(...))
> > 	return ...;
> > 
> > ?
> 
> I think it makes sense.
> 
> I wanted to preserve the error message, because the description of
> '--sort=<key>' in 'Documentation/git-ls-remote.txt' explicitly
> mentions it, and I added the condition at this place because I didn't
> want to duplicate the construction of the error message.

Ah, I didn't realize we actually documented that. And perhaps it is more
consistent, too: you'd get different results from running "ls-remote"
outside a repository versus one that just doesn't have the objects from
the other side.

> However, if we go for a more informative error message, then wouldn't
> it be better to add this condition in populate_value() before it even
> calls get_object()?  Then we could also add the problematic format
> specifier to the error message (I think, but didn't actually check),
> just in case someone specified multiple sort keys.

Yeah, that probably would be a better place. Though your response also
has made me think that maybe just sticking with the "missing object"
response is reasonable. I don't have a strong opinion between the two.

-Peff