[darcs-users] darcs patch: Make --token-chars [^ \t\n] work as advertised.

Eric Kow kowey at darcs.net
Wed Mar 18 10:36:44 UTC 2009


On Wed, Mar 18, 2009 at 13:32:29 +1100, Trent W.Buck wrote:
> This is very much a "from first priciples" solution, and I may have
> misunderstood the implications.  Therefore, it is important this patch
> is reviewed before being applied.

By as advertised, Trent is referring to this bit of documentation:

  You may prefer to define tokens in terms of delimiting characters instead of
  allowed characters using a flag such as --token-chars '[^ \n\t]', which would
  define a token as being white-space delimited.

I don't foresee the patch posing any problems in principle, but see
comments below.

Also, two requests: one, since this is something that did not work as
advertised, we should probably have a little regression test for it in
tests/.  Two (bonus points), I would sure welcome an extra patch
documenting what this function is for.

The context here being that darcs source code has insufficient
documentation but we don't have the resources to document everything.
So what I'm thinking is that the ant-trail approach could be a way to
alleviate the problem; if for whatever reason you have to touch a
function, and it doesn't already have documentation, take a few moments
out to write that documentation.  The idea is that if you had to touch
it, maybe somebody else will inevitably have to do so as well)

Make --token-chars [^ \t\n] work as advertised.
-----------------------------------------------
> Trent W. Buck <trentbuck at gmail.com>**20090318023019
>  Ignore-this: dfda7e1c8bcbd03cca26e5a473d50b6f
> ] hunk ./src/Darcs/Commands/Replace.lhs 199
>  
>  choose_toks :: [DarcsFlag] -> String -> String -> IO String
>  choose_toks (Toks t:_) a b
> -    | any isSpace t = fail $ bad_token_spec $ "Space is not allowed in the spec"
> +    | any isSpace t && '^' /= head t
> +        = fail $ bad_token_spec $ "Space is not allowed in the spec"

Style police: "head t" is liable to blow up if t is null.  If we do not
ever expect t to be null, we should probably have an error condition of
our own.  Otherwise, we should handle it.  Also, isn't the token spec
supposed to be enclosed in square brackets?  It would seem that we
should be matching on '[':'^' or "[^" `isPrefixOf` t, or something along
those lines.

For the interested, here is the full choose_toks function:

choose_toks :: [DarcsFlag] -> String -> String -> IO String
choose_toks (Toks t:_) a b
    | any isSpace t = fail $ bad_token_spec $ "Space is not allowed in the spec"
    | length t <= 2 = fail $ bad_token_spec $
                        "It must contain more than 2 characters, because " ++
                        "it should be enclosed in square brackets"
    | head t /= '[' || last t /= ']' = fail $ bad_token_spec $
                        "It should be enclosed in square brackets"
    | not (is_tok tok a) = fail $ bad_token_spec $ not_a_token a
    | not (is_tok tok b) = fail $ bad_token_spec $ not_a_token b
    | otherwise          = return tok
    where tok = init $ tail t :: String
          bad_token_spec msg = "Bad token spec: '"++ t ++"' ("++ msg ++")"
          not_a_token x = x ++ " is not a token, according to your spec"
choose_toks (_:fs) a b = choose_toks fs a b
choose_toks [] a b = if is_tok default_toks a && is_tok default_toks b
                     then return default_toks
                     else return filename_toks

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: Digital signature
URL: <http://lists.osuosl.org/pipermail/darcs-users/attachments/20090318/b86d1ab7/attachment.pgp>


More information about the darcs-users mailing list