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

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

quoted

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