Re: PATCH: allow ':/<oneline prefix>' notation to specify a specific file

[Johannes Schindelin <Johannes.Schindelin@xxxxxx>]

Hi,

On Thu, 3 Jul 2008, Junio C Hamano wrote:

> "Eric Raible" <raible@xxxxxxxxx> writes:
> 
> > This patch allows git show ":/PATCH: allow":sha1_name.c to show the
> > change to the file changed by this patch.
> > ...
> > @@ -697,8 +698,18 @@ int get_sha1_with_mode(const char *name, unsigned
> > char *sha1, unsigned *mode)
> >  		int stage = 0;
> >  		struct cache_entry *ce;
> >  		int pos;
> > -		if (namelen > 2 && name[1] == '/')
> > -			return get_sha1_oneline(name + 2, sha1);
> > +		if (namelen > 2 && name[1] == '/') {
> > +			name += 2;
> > +			colon = strrchr(name, ':');
> > +			if (!get_sha1_oneline(name, sha1) || !colon)
> > +				return 0;
> 
> So when you have ":/A:B:C", you first try to look for string "A:B:C", and
> then when it fails try "A:B" and look for path C?  I think this fallback
> makes sense, especially because this cannot break existing use for
> positive lookup (it _can_ be called a regression if you are checking to
> see if you have a commit that has A:B:C and you want the lookup to fail if
> there is A:B that happens to have path C, but I do not think we would care
> about that usage).

However, if you specify ambiguous information, you can end up with a 
commit when you expect a file.

I do not like the direction this is going; in hindsight, I think 
":/<oneline>" was a serious mistake.

As I hinted in another mail, which should have been in the same mail 
thread, "log --grep" is so much more powerful and should supersede 
":/<oneline>".

Let's grant ":/<oneline>" a quick and painless death.

Ciao,
Dscho
--
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