[darcs-users] darcs patch: Replace indexed lists by map in PatchCheck (and 1 more)

[Dmitry Kurochkin]

Hi Eric, Reinier.

Patches look good. The first patch makes code much cleaner and
simpler. The second one is tricky but does the right thing.

I think they should be applied if Reinier tested them.

Few comments:

FileContent fc_maxline is updated only in delete_line. I understand
that this is sufficient for code to work. But FileContent comment and
field name suggest that it should be updated in insert_line as well. A
bit misleading.

hunk ./src/Darcs/Patch/Check.hs 31
>  import Data.List (isPrefixOf)
>  import Control.Monad.State ( State, evalState, runState )
>  import Control.Monad.State.Class ( get, put, modify )
> +-- use Map, not IntMap, because Map has mapKeys and IntMap hasn't
> +import Data.Map ( Map )
> +import qualified Data.Map as M ( mapKeys, delete, insert, empty, lookup, null )
>

Would this comment be more useful near the use of Map,
i.e. FileContents definition?

hunk ./src/Darcs/Patch/Test.hs 49
>  import Control.Monad ( liftM, liftM2, liftM3, liftM4, replicateM )
>
>  import Darcs.Patch.Info ( PatchInfo, patchinfo )
> -import Darcs.Patch.Check ( PatchCheck, Possibly(..),
> +import Darcs.Patch.Check ( PatchCheck,
>                      check_move, remove_dir, create_dir,
>                      is_valid, insert_line, file_empty, file_exists,
>                      delete_line, modify_file, create_file, remove_file,
hunk ./src/Darcs/Patch/Test.hs 53
> -                    do_check, do_verbose_check,
> +                    do_check, do_verbose_check, FileContents(..)
>                    )
>  import RegChars ( regChars )
>  import ByteStringUtils ( linesPS )

This could be a nice one line change :)

Regards,
  Dmitry