Hi Tommy,

Excellent dogfood!

I could not apply this patch bundle because darcs was complaining that

darcs: Cannot apply this patch bundle, since we're missing:
Sat Mar 28 11:37:11 GMT 2009  Marco T�lio Gontijo e Silva 
<[email protected]>
  * Update e-mail address for Marco T�lio Gontijo e Silva.
  This patch includes an UTF-8 character in .authorspellings.

which it did indeed have.  So one question is if gzipping the patch
bundle and resending it would avoid this problem.

This sort of thing has come up before...

Also, below, I'm going to be making some really annoying meta-comments, but I
think it's useful for me to do this because in the long term, it makes the
patch history easier to use.

hadoc RegChars
--------------
Would it be a tremendous pain for you to rename this patch to "haddock
RegChars"?  Fixing typos in patch names is useful because it means we
can search for them more easily later on.

Thanks for haddocks!  If only we had something like hpc for documentation
coverage.

rollback rollback of hadock for Replace
---------------------------------------
Another patch name typo (haddock)

> ] hunk ./src/Darcs/Commands/Replace.lhs 193
>  default_toks = "A-Za-z_0-9"
>  filename_toks :: String
>  filename_toks = "A-Za-z_0-9\\-\\."
> +
> +-- | Given a set of characters and a string, returns true iff the
> +-- string contains only characters from the set.  A set beginning with
> +-- a caret (@^@) is treated as a complementary set.
>  is_tok :: String -> String -> Bool
>  is_tok _ "" = False
>  is_tok toks s = and $ map (regChars toks) s
> hunk ./src/Darcs/Commands/Replace.lhs 201
>  
> +-- | This function checks for @--token-chars@ on the command-line.  If
> +-- found, it validates the argument and returns it, without the
> +-- surrounding square brackets.  Otherwise, it returns either
> +-- 'default_toks' or 'filename_toks' as explained in 'replace_help'.
>  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"

rollback rollback of test for issue1373, and modify it
------------------------------------------------------
> Tommy Pettersson <[email protected]>**20090406205929
>  Ignore-this: 667622712a9e043d2ef526c757925d48
>  The new version of the test checks that literal spaces in complement
>  token-chars fails.

Thanks

rollback rollback of refactoring of Replace, and modify to disallow space
-------------------------------------------------------------------------
> Tommy Pettersson <[email protected]>**20090406210506
>  Ignore-this: defcc84eb4d586dd2aa1540882426773
>  
>  rolling back (part of):
>  
>  Mon Apr  6 11:38:02 CEST 2009  Eric Kow <[email protected]>
>    * Rollback issue1373 fix, which causes a regression.
>    More details in Tommy Pettersson's 2009-04-06 message on darcs-users,
>    message ID <20090406091959.ga5...@fruity>

So, I don't really care that much, but I wonder if for these kind of
patch comments, it would make sense to snip the embedded rollbacks.
That's what I do anyway.

>    rolling back:
>    
>    Sat Mar 28 10:41:48 GMT 2009  Trent W. Buck <[email protected]>
>      * Resolve issue1373: make --token-chars [^ \t\n] work as advertised.
>    
>        M ./src/Darcs/Commands/Replace.lhs -9 +20
>        A ./tests/issue1373_replace_token_chars.sh

> ] hunk ./src/Darcs/Commands/Replace.lhs 207
>  -- 'default_toks' or 'filename_toks' as explained in 'replace_help'.
>  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
> +    | length t <= 2 =
> +        bad_token_spec $ "It must contain more than 2 characters, because " 
> ++
> +                         "it should be enclosed in square brackets"
> +    | head t /= '[' || last t /= ']' =
> +        bad_token_spec "It should be enclosed in square brackets"
> +    | '^' == head tok && length tok == 1 =
> +        bad_token_spec "Must be at least one character in the complementary 
> set"
> +    | any isSpace t =
> +        bad_token_spec "Space is not allowed in the spec"
> +    | not (is_tok tok a) = bad_token_spec $ not_a_token a
> +    | not (is_tok tok b) = bad_token_spec $ not_a_token b
>      | otherwise          = return tok
>      where tok = init $ tail t :: String
> hunk ./src/Darcs/Commands/Replace.lhs 220
> -          bad_token_spec msg = "Bad token spec: '"++ t ++"' ("++ msg ++")"
> +          bad_token_spec msg = fail $ "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


add test for replace with space in token
----------------------------------------

bugfix, don't allow replace tokens containing whitespace
--------------------------------------------------------
This looks good at a glance.  Coding this stuff seems tricky because we have to
keep track (mentally) of the difference between the tokens and also the spec.

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
PGP Key ID: 08AC04F9

Attachment: signature.asc
Description: Digital signature

_______________________________________________
darcs-users mailing list
[email protected]
http://lists.osuosl.org/mailman/listinfo/darcs-users

Reply via email to