[darcs-devel] darcs patch: add test case for replacing tokens
with ... (and 3 more)
Tommy Pettersson
ptp at lysator.liu.se
Mon May 30 12:00:55 PDT 2005
On Sun, May 29, 2005 at 02:34:01PM +0100, Ian Lynagh wrote:
> > I first tried to parse the regexp on the closing ']', but that
> > got too complicated. ']' must be allowed inside '[]', and the
> > usual solution is to only allow it first (maybe after '^').
> > That's what I first tried. However, the old format allows ']'
> > anywhere (I become aware of too late), and I didn't want to
> > write a full backtracking context free parser just to solve
> > this. I'm sure there's a nifty Haskell one liner for that
> > anyway, but I took another approach. I disallow the sequence
> > "] " (closing bracket, space) in regexps. They can easily be
> > flipped to work around the limitation, and the error message
> > when this happens suggests that. Is this a fair trade-off for
> > keeping the code simple?
>
> I'm a bit lost here. If the regexp is on a line of its own, what is the
> parsing problem?
If the spaced regexp can look like the triple
<nospaceregexp,oldtok,newtok>, and the spaced oldtok on
the next line can look like the end of patch, and so on,
the parsing gets unnecessarily complicated.
> > Or shall this wait for the repo format file thing?
>
> I think this is planned to be in the next darcs release, so we may as
> well wait IMO.
>
> As an aside, I think we should probably add a version number (either
> integer or fixed point; probably fixed point 1 decimal place for
> consistency with mergers, and also because that "feels" about right) to
> all the patch types.
I'll stay tuned to supply updated patches.
> > + sal_last (ps:pss)
> > + | null pss = lastPS ps
> > + | otherwise = sal_last pss
> > + sal_last [] = error "sal_last []"
>
> FWIW, this definition seems more natural to me:
>
> sal_last [] = error "sal_last []"
> sal_last [ps] = lastPS ps
> sal_last (_:pss) = sal_last pss
>
> Nothing wrong with yours, though :-)
I can still improve a lot :-) and seeing good code helps.
> > hunk ./PatchRead.lhs 142
> > + isOldStyle s1 s2 = -- se PatchShow.showTok
> > + (sal_last s1 == ']') && (sal_head s2 /= '\n')
>
> sal_last/sal_head give errors when the input is the empty string.
> It would be better to say
> sal_take 1 (sal_reverse s1) == "]"
> sal_take 1 s2 == "\n"
> I think.
I'll take a look at it.
--
Tommy Pettersson <ptp at lysator.liu.se>
More information about the darcs-devel
mailing list