[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