[darcs-users] darcs patch: Minor refactor in Darcs.Arguments. (and 4 more)

Dmitry Kurochkin dmitry.kurochkin at gmail.com
Mon Apr 20 20:02:43 UTC 2009


Hi Eric.

All look good. Only question is on maxCount reexport. Is it a good
style? Should we move maxCount to Darcs.Arguments perhaps?

Good laziness example :) As I understand naive implementation was not
lazy because it had to go through all patches anyway to compute other
arguments of the tuple?

Detailed comments below.

Regards,
  Dmitry

Minor refactor in Darcs.Arguments.
----------------------------------
Eric Kow <kowey at darcs.net>**20090417184048

hunk ./src/Darcs/Arguments.lhs 673
>
>  __last = DarcsArgOption [] ["last"] lastn "NUMBER"
>           "select the last NUMBER patches"
> -    where lastn s = if and (map isDigit s)
> -                    then LastN (read s)
> -                    else LastN (-1)
> +    where lastn = LastN . number_string
>
>  __index = DarcsArgOption ['n'] ["index"] indexrange "N" "select one patch"
>      where indexrange s = if all isDigit s
hunk ./src/Darcs/Arguments.lhs 1565
>  patch_select_flag (AfterPattern _) = True
>  patch_select_flag (UpToPattern _) = True
>  patch_select_flag _ = False
> +
> +-- | The integer corresponding to a string, if it's only composed of digits.
> +--   Otherwise, -1.
> +number_string :: String -> Int
> +number_string s = if and (map isDigit s) then read s else (-1)
>  \end{code}

Move number parsing to a separate function. Would be used in the next patch.

Add a --max-count switch (no implementation).
---------------------------------------------
Eric Kow <kowey at darcs.net>**20090417185408

hunk ./src/Darcs/Arguments.lhs 58
>                           applyas, human_readable,
>                           changes_reverse, only_to_files,
>                           changes_format, match_one_context, match_one_nontag,
> +                         match_maxcount,
>                           send_to_context,
>                           get_context,
>                           pipe_interactive, all_interactive,
hunk ./src/Darcs/Arguments.lhs 166
>  getContent (UpToPatch s) = StringContent s
>  getContent (TagName s) = StringContent s
>  getContent (LastN s) = StringContent (show s)
> +getContent (MaxCount s) = StringContent (show s)
>  getContent (OneTag s) = StringContent s
>  getContent (AfterTag s) = StringContent s
>  getContent (UpToTag s) = StringContent s
hunk ./src/Darcs/Arguments.lhs 692
>                           else PatchIndexRange 0 0
>            isokay c = isDigit c || c == '-'
>
> +match_maxcount :: DarcsOption
> +match_maxcount = DarcsArgOption [] ["max-count"] mc "NUMBER"
> +         "return only NUMBER results"
> +    where mc = MaxCount . number_string
> +
> +
>  -- | 'get_context' takes a list of flags and returns the context
>  -- specified by @Context c@ in that list of flags, if any.
>  -- This flag is present if darcs was invoked with @--context=FILE@

Add match_maxcount option.

hunk ./src/Darcs/Flags.hs 18
>  -- the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
>  -- Boston, MA 02110-1301, USA.
>
> -module Darcs.Flags ( DarcsFlag( .. ), Compression( .. ), compression, want_external_merge, isInteractive) where
> +module Darcs.Flags ( DarcsFlag( .. ), Compression( .. ), compression, want_external_merge, isInteractive,
> +                     maxCount,
> +                   ) where
>  import Darcs.Patch.MatchData ( PatchMatch )
>  import Darcs.RepoPath ( AbsolutePath, AbsolutePathOrStd )
>
hunk ./src/Darcs/Flags.hs 37
>                 | SendmailCmd String | Author String | PatchName String
>                 | OnePatch String | SeveralPatch String
>                 | AfterPatch String | UpToPatch String
> -               | TagName String | LastN Int | PatchIndexRange Int Int
> +               | TagName String | LastN Int | MaxCount Int | PatchIndexRange Int Int
>                 | NumberPatches
>                 | OneTag String | AfterTag String | UpToTag String
>                 | Context AbsolutePath | Count

Add MaxCount flag.

hunk ./src/Darcs/Flags.hs 112
>        isInteractive_ _ (DryRun:fs) = isInteractive_ False fs
>        isInteractive_ def (_:fs) = isInteractive_ def fs
>
> +maxCount :: [DarcsFlag] -> Maybe Int
> +maxCount (MaxCount n : _) = Just n
> +maxCount (_:xs) = maxCount xs
> +maxCount [] = Nothing

macCount function to get --max-count parameter from list of options.

resolve issue1437: Implement darcs changes --max-count.
-------------------------------------------------------
Eric Kow <kowey at darcs.net>**20090417185959

hunk ./src/Darcs/Arguments.lhs 25
>  #include "gadts.h"
>
>  module Darcs.Arguments ( DarcsFlag( .. ), flagToString,
> +                         maxCount,
>                           isin, arein,
>                           definePatches, defineChanges,
>                           fixFilePathOrStd, fixUrl,
hunk ./src/Darcs/Arguments.lhs 116
>                          ioAbsolute, ioAbsoluteOrStd,
>                          makeAbsolute, makeAbsoluteOrStd, rootDirectory )
>  import Darcs.Patch.MatchData ( patch_match )
> -import Darcs.Flags ( DarcsFlag(..) )
> +import Darcs.Flags ( DarcsFlag(..), maxCount )
>  import Darcs.Repository ( slurp_pending, withRepository, ($-) )
>  import Darcs.Repository.HashedRepo ( slurp_all_but_darcs )
>  import Darcs.SlurpDirectory ( list_slurpy )

Reexport maxCount from Darcs.Flags. Is this a good style?

hunk ./src/Darcs/Commands/Changes.lhs 42
>                           working_repo_dir, only_to_files,
>                           summary, changes_reverse,
>                           match_several_or_range,
> +                         match_maxcount, maxCount,
>                           all_interactive, showFriendly,
>                           network_options
>                        )
hunk ./src/Darcs/Commands/Changes.lhs 93
>                          command_argdefaults = nodefaults,
>                          command_advanced_options = network_options,
>                          command_basic_options = [match_several_or_range,
> +                                                 match_maxcount,
>                                                   only_to_files,
>                                                   changes_format,
>                                                   summary,

Changes now accepts --max-count.

hunk ./src/Darcs/Commands/Changes.lhs 151
>  Without any options to limit the scope of the changes, history will be displayed
>  going back as far as possible.
>
> +\begin{options}
> +--max-count
> +\end{options}
> +
> +If changes is given a \verb!--max-count! option, it only outputs up to as that
> +number of changes.
>
>  \begin{code}
>  get_changes_info :: RepoPatch p => [DarcsFlag] -> [FilePath] -> PatchSet p

Docs.

hunk ./src/Darcs/Commands/Changes.lhs 163
>                   -> ([PatchInfoAnd p], [FilePath], Doc)
>  get_changes_info opts plain_fs ps =
>    case get_common_and_uncommon (p2s,p1s) of
> -  (_,us:\/:_) -> filter_patches_by_names fs $ filter pf $ unsafeUnRL $ concatRL us
> +  (_,us:\/:_) -> mxcount $ filter_patches_by_names fs $ filter pf $ unsafeUnRL $ concatRL us
>    where fs = map (\x -> "./" ++ x) $ plain_fs
>          p1s = if first_match opts then unsafeUnseal $ match_first_patchset opts ps
>                                    else NilRL:<:NilRL
hunk ./src/Darcs/Commands/Changes.lhs 172
>          pf = if have_nonrange_match opts
>               then match_a_patchread opts
>               else \_ -> True
> +        mxcount = maybe id (first3 . take) $ maxCount opts
> +        first3 f (a,b,c) = (f a, b, c)
>
>  filter_patches_by_names :: RepoPatch p => [FilePath]
>                          -> [PatchInfoAnd p]

Take only first maxCount elements from PatchInfoAnd list.

Push the max-count handling into filter_patches_by_names to allow for laziness.
-------------------------------------------------------------------------------
Petr Rockai <me at mornfall.net>**20090419120939

hunk ./src/Darcs/Commands/Changes.lhs 163
>                   -> ([PatchInfoAnd p], [FilePath], Doc)
>  get_changes_info opts plain_fs ps =
>    case get_common_and_uncommon (p2s,p1s) of
> -  (_,us:\/:_) -> mxcount $ filter_patches_by_names fs $ filter pf $ unsafeUnRL $ concatRL us
> +  (_,us:\/:_) -> filter_patches_by_names (maxCount opts) fs $ filter pf $ unsafeUnRL $ concatRL us
>    where fs = map (\x -> "./" ++ x) $ plain_fs
>          p1s = if first_match opts then unsafeUnseal $ match_first_patchset opts ps
>                                    else NilRL:<:NilRL
hunk ./src/Darcs/Commands/Changes.lhs 172
>          pf = if have_nonrange_match opts
>               then match_a_patchread opts
>               else \_ -> True
> -        mxcount = maybe id (first3 . take) $ maxCount opts
> -        first3 f (a,b,c) = (f a, b, c)
>

Pass maxCount as a parameter to filter_patches_by_names.

hunk ./src/Darcs/Commands/Changes.lhs 173
> -filter_patches_by_names :: RepoPatch p => [FilePath]
> +filter_patches_by_names :: RepoPatch p => Maybe Int -> [FilePath]
>                          -> [PatchInfoAnd p]
>                          -> ([PatchInfoAnd p],[FilePath], Doc)
hunk ./src/Darcs/Commands/Changes.lhs 176
> -filter_patches_by_names _ [] = ([], [], empty)
> -filter_patches_by_names [] pps = (pps, [], empty)
> -filter_patches_by_names fs (hp:ps)
> +filter_patches_by_names (Just 0) _ _ = ([], [], empty)
> +filter_patches_by_names _ _ [] = ([], [], empty)
> +filter_patches_by_names _ [] pps = (pps, [], empty)
> +filter_patches_by_names maxcount fs (hp:ps)
>      | Just p <- hopefullyM hp =
>      case look_touch fs (invert p) of
>      (True, []) -> ([hp], fs, empty)
hunk ./src/Darcs/Commands/Changes.lhs 183
> -    (True, fs') -> hp -:- filter_patches_by_names fs' ps
> -    (False, fs') -> filter_patches_by_names fs' ps
> -filter_patches_by_names _ (hp:_) =
> +    (True, fs') -> hp -:- filter_patches_by_names
> +                             (subtract 1 `fmap` maxcount) fs' ps
> +    (False, fs') -> filter_patches_by_names maxcount fs' ps
> +filter_patches_by_names _ _ (hp:_) =
>      ([], [], text "Can't find changes prior to:" $$ description hp)
>
>  (-:-) :: a -> ([a],b,c) -> ([a],b,c)

Handle maxCount in filter_patches_by_names.

Haddock filter_patches_by_name, sort of.
----------------------------------------
Petr Rockai <me at mornfall.net>**20090419120943

hunk ./src/Darcs/Commands/Changes.lhs 173
>               then match_a_patchread opts
>               else \_ -> True
>
> -filter_patches_by_names :: RepoPatch p => Maybe Int -> [FilePath]
> -                        -> [PatchInfoAnd p]
> +-- | Take a list of filenames and patches and produce a list of patches that
> +-- actually touch the given files, a new file list that represents the same set
> +-- of files as in input, before the returned patches would have been applied,
> +-- and possibly an error. Additionaly, the function takes a "depth limit" --
> +-- maxcount, that could be Nothing (return everything) or "Just n" -- returns
> +-- at most n patches touching the file (starting from the beginning of the
> +-- patch list).
> +filter_patches_by_names :: RepoPatch p =>
> +                           Maybe Int -- ^ maxcount
> +                        -> [FilePath] -- ^ filenames
> +                        -> [PatchInfoAnd p] -- ^ patchlist
>                          -> ([PatchInfoAnd p],[FilePath], Doc)
>  filter_patches_by_names (Just 0) _ _ = ([], [], empty)
>  filter_patches_by_names _ _ [] = ([], [], empty)

Docs.


More information about the darcs-users mailing list