[darcs-devel] FastPackedString issues

[David Roundy]

On Fri, Aug 26, 2005 at 03:57:39PM +1000, Donald Bruce Stewart wrote:
> droundy:
> > On Thu, Aug 25, 2005 at 02:47:37PM +1000, Donald Bruce Stewart wrote:
> > > While writing some unit tests for FastPackedString, I found two issues.
> > > The first problem is initPS. It seems to have an off-by-one error:
> > 
> > Indeed, this does look like a bug.  I'd say a more thorough solution might
> > be to eliminate the (confusing) substrPS function.  It is only used twice
> > and one of those uses is buggy! And it's not exported from
> > FastPackedString.  I think initPS and tailPS will be much more clearly
> > written as
> 
> ...
> 
> Good idea. I've now removed substrPS, and added your initPS/tailPS
> implementations.

Could you send another patch?

> > > The second is that linesPS/unlinesPS aren't equivalent to
> > > lines/unlines.
> > 
> > For non-darcs usage, it might be nice to mimic the behavior of lines
> > and unlines, but in my opinion the linesPS behavior is much more
> > useful, although somewhat less intuitive.
> 
> Ok, maybe we should keep the darcs-style unlinesPS/linesPS, but with
> alternate names, so as not to confuse people expecting line/unlines
> behaviour. Any suggestions?

Another possibility would be to query the libraries list to see if they'd
consider modifying the prelude (which is perhaps unlikely).  On the other
hand, the comment in the haskell report is false, so one could decide to
revise the code in favor of the comment rather than the other way around:

>From the Prelude:

-- lines breaks a string up into a list of strings at newline characters.
-- The resulting strings do not contain newlines.  Similary, words
-- breaks a string up into a list of words, which were delimited by
-- white space.  unlines and unwords are the inverse operations.
-- unlines joins lines with terminating newlines, and unwords joins
-- words with separating spaces.

It claims that ulines is the inverse of lines... of course it also says the
same thing about unwords, which is even more untrue...  I guess they
probably wouldn't be keen on modifying the Prelude, but it might not hurt
to ask.

> Also, I see: 
>     Prelude Data.FastPackedString> words "a          b"
>     ["a","b"]
>     Prelude Data.FastPackedString> wordsPS (packString "a          b")
>     ["a","","","","","","","","","","b"]
> 
> The difference is:
>     wordsPS ps = filter (not.nullPS) (splitWithPS isSpace ps)
> versus:
>     wordsPS ps = splitWithPS isSpace ps
> 
> So you've picked a different wordsPS implementation too?

This one was probably accidental.  wordsPS is never used in darcs, and I
wouldn't mind making it equivalent to the Prelude words.  Making wordsPS
invertible seems less useful than making linesPS invertible.
-- 
David Roundy
http://www.darcs.net