Florian Kberle <FloriansKarten@web.de> wrote:
quoted text
> | This is an interesting start.  Did you see the existing "Main" class
> | in org.spearce.jgit/src/org/spearce/jgit/pgm?  It sets up and invokes
> | a TextBuiltin, which is sort of like the "Command" class you added in
> | your first patch.  Though TextBuiltins are created on-the-fly and thus
> | are harder/impossible to use to format a "jgit help".
>
> I noticed that the class appeared after a rebase, but didn't have a
> closer look to it yet.

I guess you started working on an older version, only to later find
out that I had also done a lot of work in the mean-time.  :-)

My jgit contributions come in huge bursts.  RevWalk/TreeWalk was one
back in March; the transport API (almost 10,000 lines of code itself)
is the most recent from late April/early May.  Once its fully into
the mainline I'll probably have to slow down for a couple of months.
I have to move in July and have a lot of things to do between now
and then.
 
quoted text
> | Please note that jgit is restricted to Java 5 APIs only right now.
> | The "MainProgram" class you introduced uses Arrays.copyOfRange()
> | which does not compile under Java 5.  I guess it is new in Java 6?
>
> Yes it is new in Java 6. A patch fixing this is contained in the patch
> set I send to the mailing list.

To keep the history bisectable as much as possible it is better
if you use `git rebase -i` to squash these two changes together,
so that we never introduce the Java 6 usage into the codebase.

quoted text
> I had a look at the WorkingTreeIterator and it seems to me that it is
> possible to reuse my Rules class there.
> 
> We could simply give the iterator a member variable of type Rule.
> 
> The method loadEntries of WorkingTreeIterator could then use the rules
> class to filter out unwanted files and directories.

Yea, that sounds right.
 
quoted text
> Also note that my Rules implementation would ignore the directory a in
> the case of "/a\n!/a/b.txt". This means that a directory may not appear
> in the list entries, but must be used to create another iterator.

Ouch.  I forgot about that fun corner case.  In the context of a
TreeWalk directory "a" must actually still be reported as an entry so
that the TreeWalk main loop knows to enter into the subtree iterator.
However the subtree iterator needs to only have entry "b.txt" within
its entry list.
 
quoted text
> I suggest to put all the classes from the package
> org.spearce.jgit.treewalk and the package
> org.spearce.jgit.lib.fileiteration into one package. Please tell me
> which package and I will send a patch, or do it yourself. I don't have
> any outstanding changes.

The treewalk package is already established so I would say add
them there.  Since you are the original developer and your code is
not yet in mainline I would ask that you perform the renames.

quoted text
> I don't see a easy way of porting my Rules implementation to the
> TreeFilter framework, but as you may noticed it is may not necessary to
> do so.

The TreeFilter framework is perhaps not the right API for ignore
rules, that is likely true.  It also works with paths as byte[] and
not as String, because we get byte[] (generally UTF-8 encoded) data
from canonical tree objects when reading from the object database.
Avoiding the conversion for most entries is a huge performance
improvement for us.

I still won't give up my silly dream for `jgit log -- 'foo/*.c'`,
but maybe we do need two different implementations to make things
work out well.

-- 
Shawn.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html