Hey David, Tommy, all. David, could you have a second look at these?
> The result is that now a Repository "knows" if it should be ignoring > the time stamps or not. This refactoring could lead to others, since > there are certainly functions that should no longer need to have the > [DarcsFlag] passed. Yeah, not having both sounds good, but... In the Repository.lhs module, there is a get_unrecorded function that now has oldopts and moreopts. Can moreopts go away? Also, I'm slightly unhappy about passing [] to identifyRepository in cases where the flags are irrelevant. I don't have a clear objections to it just misgivings about it coming back to haunt us in the future. Maybe it's a more general feeling that passing flags around just isn't explicit enough for Repository? I was considering maybe a Bool or a record, but this seems also inconvenient and messy because you'd have to convert from the flags. As a slight consolation, I'll note that Diff.gendiff uses a tuple of Bools, so if we were to come up with some kind of record, maybe we could refactor against that. > Thu Jan 25 07:38:03 PST 2007 Tommy Pettersson <[EMAIL PROTECTED]> > * add test for replace that messes with unrecorded hunks > echo "x" > foo > darcs rec -am x > echo "y" > foo > darcs replace x y foo > > # this fails > echo "hej" > foo > darcs rec -am hej > echo "hopp" > foo > darcs replace hej hopp foo Thanks for the test! If I'm reading this right, these are the same test but just repeated so that we can slip into the condition where it fails, right? There is nothing about the first test that makes it any less prone to failure (buggy pending) than the second one, is there? Would it maybe make sense to capture this by having the same test repeated in a for loop, say "darcs replace hej$x hopp$x" where $x is a counter? Or rather, maybe just keeping one with --ignore-times; would that do the trick? > Sat Jan 27 16:07:28 PST 2007 David Roundy <[EMAIL PROTECTED]> > * refactor: add opts into Repository. See random apprehensions above. Actually, I was going to take the patch, but since I am rejecting the bugfix, I am in no hurry. > Sat Jan 27 16:18:26 PST 2007 David Roundy <[EMAIL PROTECTED]> > * fix bugs in replace.sh script--running wrong darcs. Yep. Going in. > Sat Jan 27 16:22:06 PST 2007 David Roundy <[EMAIL PROTECTED]> > * fix bug triggered in replace.sh I think this one introduces a bug: # so far so good darcs init echo hej > a darcs add a darcs record -am hej # uh-oh sed -e s/hej/xxx/ a > a.tmp; mv -f a.tmp a darcs replace --ignore-times xxx hej a This succeeds, and I don't think it should. I believe the problem is that we've become too permissive in that we treat the apply suceeding in working as being good enough when we should really be checking that it suceeds in both working and pristine. Also, I instinctively agree that replace should accept --ignore-times, but I don't understand where it's getting used. If it is getting used, should it maybe be used all the time? Unrelated: I notice also a smart_diff in the Replace code. Should it be paranoid_diff, rather? -- Eric Kow http://www.loria.fr/~kow PGP Key ID: 08AC04F9 Merci de corriger mon français.
pgpbVqlt4mvYx.pgp
Description: PGP signature
_______________________________________________ darcs-devel mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-devel
