[darcs-users] Fix for issue 1473

[Eric Kow]

Hi Florian,

> Sat Aug 22 08:54:31 CEST 2009  Florian Gilcher <flo at andersground.net>
>   * Switched pattern order in Population.lookup_pop' to ensure that
>   --repodir is honored if directory name is ".". Fixes #1473.

I've very pleased to see that you got a chance to work on this.

First some minor things to address:
 - patch short name should be 72 characters or less
 - start the patch name with "resolve issue1473:".
   It tells our roundup-darcs integration script to automatically update
   the bugtracker status
 - write a regression test for this bug

You may be interested in http://wiki.darcs.net/DeveloperGettingStarted
for more details

Unfortunately, I wasn't able to finish reviewing this patch, but I'll
note down what I have now, hoping that that somebody can pick up where I
left off.

>  lookup_pop' d p@(Pop pinfo (PopDir i c))
> - -    | BC.unpack (nameI i) == "." =
> - -        case catMaybes $ map (lookup_pop' (dropDS d).(Pop pinfo)) c of
> - -        [apop] -> Just apop
> - -        [] -> Nothing
> - -        _ -> impossible
>      | BC.unpack (nameI i) == takeWhile (/='/') d =
>          case dropWhile (=='/') $ dropWhile (/='/') d of
>          "" -> Just p

Background: a Population is a 'simpler' representation of a file tree
used by annotate.  It seems to associate each file with some tracking
data: the name, the last match that touched it, etc.  The lookup_pop
function seems to retrieves the subtree that corresponds to a given
filename (Looks like this just returns a one-node tree; perhaps that's a
chance to narrow some code down if so).  From what I can tell, the
helper function lookup_pop' is just a simultaneous recursive traversal:
each iteration simultaneously consumes a piece of the path (foo/bar/baz
=> bar/baz) and descends one node down the Population tree.

Florian's patch fixes a bug where darcs crashes if you give '.' as
a path while also giving a repodir argument.

I still don't have a very good idea why this patch works exactly.

Highlighting just the cases below, I think the first case is for the
root of the population whereas the second case helps us walk one level
deeper.  How swapping their order helps still isn't clicking yet :-)

Thanks!

>  lookup_pop' d p@(Pop pinfo (PopDir i c))
>      | BC.unpack (nameI i) == "." =
>      | BC.unpack (nameI i) == takeWhile (/='/') d =

PS. This may also be a good chance to throw in some haddock if you
    think it'll be useful to other developers working on the same
    code.  We want to keep polishing, each modification we make
    making the darcs code just a tiny bit more accessible.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 194 bytes
Desc: not available
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090823/ce3ef9da/attachment.pgp>