On Tue, Sep 9, 2008 at 2:01 PM, Eric Kow <[EMAIL PROTECTED]> wrote:
> Here are some patches from David which appeared in unstable.  I need a
> volunteer to review them.  Takers?

I don't know enough about the path module to really comment on David's
bug fixes, but a few things do stick out to me in this module.

Since we cleanup paths in some places, we should store clean and
unclean paths in different types so that the type system can tell us
which is expected where.

For example, we might export RawPath constructor but not the CleanPath
constructor:
newtype RawPath = RP String
newtype CleanPath = CP String

Then we make these available:
cleanup :: RawPath -> CleanPath
cleanPathToString :: CleanPath -> String

Or we could generalize it to store the different path types we have,
but the basic idea should make sense.  We could even use String in
place of RawPath.

We could go a step further and manipulate paths as lists that we join
back together with the proper separator when it's time to use the
path.

>
> Tue Sep  9 16:53:25 BST 2008  David Roundy <[EMAIL PROTECTED]>
>  * fix bug in file path handling.

I was confused by ioAbsolute when I first saw it.  I didn't know what
was absolute about IO.  The type makes it more clear, but I think the
function name could be improved.

What does (/-) do?  Looking at the first case, it seems to remove a
leading slash from the second parameter.  Looking at the last two
cases, it seems to add a leading slash to the second parameter and
concatenate with the first parameter.  If so, I think this may need a
separation of steps.  One step seems to be reducing a to single
leading slash and the other joining path elements with slashes.  This
strikes me as a place where we could have:
(/-) :: AbsolutePath -> CleanPath -> AbsolutePath

And maybe then, simpleClean :: String -> CleanPath

Although, it does seem we do different cleanups in different places.
For example, simpleClean would nuke the trailing slashes but the way
we clean the arguments to (/-) we don't currently do that.
Unfortunately it's not documented why/when we do which.

More haddocks would be nice too.

>
> Tue Sep  9 16:54:13 BST 2008  David Roundy <[EMAIL PROTECTED]>
>  * don't inline global variables in URL.

No comment.

>
> Tue Sep  9 16:55:23 BST 2008  David Roundy <[EMAIL PROTECTED]>
>  * resolve issue1057: this was fixed in the previous patch.

Again no comment.

Thanks,
Jason
_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to