LGTM.

On Mon, Apr 20, 2009 at 12:53 PM, Eric Ayers <[email protected]> wrote:
>
>
> On Mon, Apr 20, 2009 at 3:46 PM, Ian Petersen <[email protected]> wrote:
>>
>> On Mon, Apr 20, 2009 at 12:36 PM, Eric Ayers <[email protected]> wrote:
>> > Here is a new patch that incorporates the test for $GIT_DIR.  I don't
>> > know
>> > if 'index' is the right directory to test for.  It is present both in
>> > git
>> > repositories that support SVN and those that don't.
>>
>> I was testing for "index" to make sure the directory looks like a Git
>> directory rather than it being some arbitrary directory that just
>> happens to be named .git.  If svn is a better thing to look for,
>> that's great.
>>
>> As for the patch, there's a problem here:
>>
>> +    dir = dir.getAbsoluteFile();
>> +    while (dir != null) {
>> +      if (new File(dir, ".git").isDirectory()) {
>> +        return dir;
>> +      }
>> +      dir = dir.getParentFile();
>> +    }
>>
>> My (undocumented) assumption is that findGitDir(File) would return the
>> .git directory, not the directory containing the .git directory.
>> Currently, the "look at $GIT_DIR" code path returns the equivalent of
>> the .git dir while the "recurse up the tree" code path returns the dir
>> that contains the .git dir.  To fix it, you need to change the above
>> return statement to
>>
>> return new File(dir, ".git")
>
> Thanks Ian,  that was a bug with me trying to merge those 2 versions of the
> code.
>
>  while (dir != null) {
>       File gitDir = new File(dir, ".git");
>       if (gitDir.isDirectory()) {
>         return gitDir;
>       }
>       dir = dir.getParentFile();
>     }
>
>
>>
>> Otherwise, LGTM.
>>
>> Ian
>>
>> >
>> > On Mon, Apr 20, 2009 at 2:28 PM, Scott Blum <[email protected]> wrote:
>> >>
>> >> This looks good as far as I can eyeball. �...@eric: what do you think?
>> >>
>> >> On Mon, Apr 20, 2009 at 2:18 PM, Ian Petersen <[email protected]>
>> >> wrote:
>> >>>
>> >>> On Mon, Apr 20, 2009 at 10:59 AM, Scott Blum <[email protected]>
>> >>> wrote:
>> >>> > That would be a nice enhancement.  Patches welcome. :)
>> >>>
>> >>> How about this?
>> >>>
>> >>> static boolean looksLikeGit(File dir) {
>> >>>  if (looksLikeSvn(dir)) {
>> >>>    // Prefer svn to git.
>> >>>    return false;
>> >>>  }
>> >>>
>> >>>  File gitDir = findGitDir(dir);
>> >>>
>> >>>  return gitDir != null && new File(gitDir, "index").isFile();
>> >>> }
>> >>>
>> >>> static File findGitDir(File dir) {
>> >>>  File ret;
>> >>>
>> >>>  // try $GIT_DIR
>> >>>  // is there any point in worrying about SecurityExceptions here?
>> >>>  // if this code lacks the ability to access $GIT_DIR, it would
>> >>>  // maybe make sense to fall back on the default below.
>> >>>  String gitDir = System.getenv("GIT_DIR");
>> >>>
>> >>>  if (gitDir != null) {
>> >>>    ret = new File(gitDir).getAbsoluteFile();
>> >>>
>> >>>    if (ret.isDirectory()) {
>> >>>      return ret;
>> >>>    }
>> >>>  }
>> >>>
>> >>>  // try searching for a .git at or above dir
>> >>>  dir = dir.getAbsoluteFile();
>> >>>  while (dir != null) {
>> >>>    ret = new File(ret, ".git");
>> >>>    if (ret.isDirectory()) {
>> >>>      return ret;
>> >>>    }
>> >>>    dir = dir.getParentFile();
>> >>>  }
>> >>>
>> >>>  return null;
>> >>> }
>> >>>
>> >>> That was typed directly into email and I haven't written any Java in
>> >>> half a year, so at least compile it before doing anything serious.
>> >>> Also, it could probably do with a refactoring but that's a real pain
>> >>> in a text area.
>> >>>
>> >>> Ian
>> >>>
>> >>>
>> >>
>> >>
>> >>
>> >
>> >
>> >
>> > --
>> > Eric Z. Ayers - GWT Team - Atlanta, GA USA
>> > http://code.google.com/webtoolkit/
>> >
>> > >
>> >
>>
>>
>
>
>
> --
> Eric Z. Ayers - GWT Team - Atlanta, GA USA
> http://code.google.com/webtoolkit/
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
http://groups.google.com/group/Google-Web-Toolkit-Contributors
-~----------~----~----~----~------~----~------~--~---

Reply via email to