[darcs-users] darcs patch: split FileName into two modules. (and 1 more)

David Roundy droundy at darcs.net
Fri Sep 26 21:22:13 UTC 2008


On Fri, Sep 26, 2008 at 06:08:33PM +0100, Eric Kow wrote:
> On Fri, Sep 26, 2008 at 11:03:29 -0400, David Roundy wrote:
> > I'd like to get this into 2.1, just because I'm not entirely
> > comfortable with how Dmitry's change to drop_dotdots might affect the
> > darcs internals.  This way we can keep the patch theory itself frozen
> > unless bugs are found, while still reworking the UI.  And yes,
> > UglyFileName is a stupid name.  It's supposed to be stupid, so noone
> > will want to use it.
> 
> Yep! This patch is straightforward enough, doesn't actually change
> anything... so I would be willing to put it in the release.

Great.

> Some quibbling below about where the slice the baby.

explanations of decisions (some of which were made very quickly)...

First note:  UglyFileName no longer exports the FileName type, so
it can (quite correctly) only be used to deal with other sorts of
types.  I trimmed both export lists of everything I could think of,
but may have missed some functions.  Further trimmings would be
appreciated! :)

> Darcs.Patch.FileName:
> > hunk ./src/Darcs/Commands/Add.lhs 43
> > hunk ./src/Darcs/Commands/Changes.lhs 44
> > hunk ./src/Darcs/Commands/Mv.lhs 43
> > hunk ./src/Darcs/FilePathMonad.lhs 27
> > hunk ./src/Darcs/Match.lhs 59
> > hunk ./src/Darcs/Patch/Prim.lhs 55
> > hunk ./src/Darcs/Patch/QuickCheck.lhs 33
> > hunk ./src/Darcs/Patch/Test.lhs 58
> > hunk ./src/Darcs/Population.lhs 39
> > hunk ./src/Darcs/Patch/Commute.lhs 37
> > hunk ./src/Darcs/Patch/Core.lhs 37
> > hunk ./src/Darcs/Repository/Prefs.lhs 46
> 
> UglyFileName [DEPRECATED]:
> > hunk ./src/Darcs/Commands/Dist.lhs 34
> > hunk ./src/Darcs/Commands/Send.lhs 65
> > hunk ./src/Darcs/CommandsAux.lhs 24
> > hunk ./src/Darcs/External.hs 67
> > hunk ./src/Darcs/FilePathUtils.hs 27 [DEPRECATED]
> > hunk ./src/Darcs/Lock.lhs 64
> 
> Both:
> > hunk ./src/Darcs/RepoPath.hs 36 [Ugly, but D.P.F for SubPath]

Yeah, this one should be the first to have the Ugly import removed.
The D.P.F import is correct, and in fact defines probably the most
correct way to construct a D.P.FileName.  It might be wise to move
this function into D.P.F., if that won't cause import loops.  Then we
could eventually remove the export of fp2fn, which would be nice.

> Unsure:
> > hunk ./src/Darcs/IO.lhs 60 (using Darcs.Patch.FileName)

This definitely needs to be like this, since it's our primary means of
interacting with slurpies and patches.  We could move it to
Darcs.Patch.IO, but that'd just be quibbling, I think.

> > hunk ./src/Darcs/Patch/Apply.lhs (D.P.FileName... but maybe both?)

Why is this vague? This is one of the primary reasons wy D.P.FileName
exists, and it absolutely needs to use it.

> > hunk ./src/Darcs/Repository/HashedIO.lhs
> 
> Maybe this should also be importing both modules?  When I see
> Darcs.Patch.FileName, I expect us to only manipulate paths which we
> would see in a patch.  But here we seem to be using it for all sorts
> of subpaths, for example, for _darcs/patch/foo, which sound more
> like they are on the Ugly side of the fence.

Again, HashedIO is a concrete implementation of Darcs.IO, which is in
our little FileName world.  It may be that HashedIO also does
something inappropriate, but if so, I don't see it.

> > hunk ./src/Darcs/Repository/HashedRepo.lhs 62
> 
> Likewise?

Here it's used soley for slurpy interaction, which is precisely what
it should be used for.  Although, it'd be nice if we could eliminate
all use of slurp_all_but_darcs, which is something of an ugly
function.

> > hunk ./src/Darcs/Repository/Pristine.lhs 51
> 
> Potentially ugly if we consider _darcs/pristine to be an ugly filename
> (again, I'm not looking closely, maybe we're only touching tree paths)

It's only used for applyPristine, which is a correct use.

> Other comments:
> 
> > +module Darcs.Patch.FileName ( FileName( ),
> > +                              fp2fn, fn2fp,
> > +                              fn2ps, ps2fn,
> > +                              niceps2fn, fn2niceps,
> > +                              break_on_dir, norm_path, own_name, super_name,
> > +                              movedirfilename,
> > +                              encode_white, decode_white,
> > +                              (///),
> > +                            ) where
> 
> Should we scrutinise this export list and get rid of some unused stuff?

I did that a bit, but may have missed something.

David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
Url : http://lists.osuosl.org/pipermail/darcs-users/attachments/20080926/e986255e/attachment.pgp 


More information about the darcs-users mailing list