Re: [PATCH] sha1_name(): accept ':directory/' to get at the cache_tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On Tue, 9 Jan 2007, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
> 
> > 	'tis a resend of an earlier patch, but without support for the
> > 	bogus ":." as equivalent to ":/".
> >
> > 	I find this feature highly convenient when I just want to see
> > 	what files the index contains.
> 
> I do not understand; do you mean ls-files?

It is the same as ls-files, except "show" is shorter, is labeled as 
porcelain, easier to remember, _and_ it runs the pager automatically.

> In any case, I wonder if this does a sane thing if you asked
> "git show :3:t/" on a fully merged index.

No. That slipped through. The code will just ignore the stage (if 
specified) for trees.

> > @@ -561,6 +562,23 @@ int get_sha1(const char *name, unsigned char *sha1)
> >  			}
> >  			pos++;
> >  		}
> > +		if (namelen > 0 && cp[namelen - 1] == '/')
> > +			namelen--;
> > +		if (namelen == 0 || (ce && ce_namelen(ce) > namelen &&
> > +					ce->name[namelen] == '/' &&
> > +					!memcmp(ce->name, cp, namelen))) {
> 
> I may be misreading the code, but what does ce point at?  Does
> this get the index sort order correctly?

It is still at what cache_name_pos(cp, namelen) pointed to, or NULL if 
the entry does not exist (e.g. there is no cache entry, or all cache 
entries are lexicographically smaller).

> For example, would this work?
> 
> 	$ echo >t- && git add t-
>         $ git show :t
> 	$ git show :t/

I think so: The code explicitely checks for a trailing '/'. (See second 
last line of the hunk you quoted.)

> > +			struct cache_tree *tree =
> > +				cache_tree_find(active_cache_tree, cp);
> > +			if (!cache_tree_fully_valid(tree)) {
> > +				ret = cache_tree_update(active_cache_tree,
> > +						active_cache, active_nr, 0, 0);
> > +				if (ret < 0)
> > +					return ret;
> 
> This gracefully errs out when the index is unmerged but fails to
> pretend the index knows about trees, if the unmerged part of
> index is outside the directory the user specified.

That is correct. But in that case, we cannot sanely ask the question "what 
would the tree object look like if we committed right now?"

> In short, I am not sure if it is worth it, and especially if the 
> motivation is to pretend as if the index contains trees, I would be 
> opposed to it.  The index does _not_ contain trees, and cache-tree is a 
> pure optimization for the next write-tree. Nothing more.

Well, it _is_ the method to construct tree objects for committing them.

> If it (pretending as if the index contains trees) is just a means to 
> achieve something else worthy, I would not necessarily oppose to that 
> goal, but I do not see what it is, and I do not know if the approach is 
> right...

I wanted to have it for teaching purposes. And maybe a little bit for 
completeness.

But then it turned out that I even use it before committing, like when I 
ask "what files would be in my next revision?" It is not a question 
arising daily, but sometimes it is interesting to see _before_ committing 
them. (Note that I am not only interested in the _modified_ files.)

But if you feel strongly about that feature, don't take the patch.

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

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]