On Tue, Sep 09, 2008 at 03:42:40PM -0700, Jason Dagit wrote: > 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.
Yes, we do store cleaned paths in separate types for that reason, we combine this with ensuring that paths are absolute (or relative). > > 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. Yes, a better name would be better. > 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 It's only used once, and isn't exported. Perhaps I should have made it local to the function where it's used. It does assume a cleaned String as its second argument. Better than CleanPath would be to use SubPath, but I didn't want to make things that complicated, since it's only used once. I think it would be best to clean this up (and maybe give it a better name, I chose /- because /// is overused, and this is intended to be an asymmetrical version of /// where the first path is always absolute and the second never is). If we do improve this function and export it, we'd need to decide whether to keep the second argument as String and clean it up, or to make the second argument SubPath. Making the second argument SubPath would be prettier, but would make the transition to using AbsolutePath harder. David _______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
