[darcs-users] [patch146] tweaks to hunk editing interface

Florent Becker bugs at darcs.net
Tue Jan 19 15:46:48 UTC 2010


Florent Becker <florent.becker at ens-lyon.org> added the comment:

tweaks to hunk editing interface
--------------------------------
Ganesh Sittampalam <ganesh at earth.li>**20100118205754

hunk ./src/Darcs/Patch/Split.hs 63
>  -- splitters for Prim etc, and the generality doesn't cost anything.
>  data Splitter p
>    = Splitter {
> -              applySplitter :: FORALL(x y) p C(x y)
> -                              -> Maybe (B.ByteString,
> -                                        B.ByteString -> Maybe (FL p C(x y)))
> +              applySplitter :: FORALL(x y) p C(x y) -- ^patch to split
> +                            -> Maybe (B.ByteString, -- ^text to present for
editing
> +                                      B.ByteString -> Maybe ((FL p :> FL p :>
FL p) C(x y))
> +                                       -- ^parse edited text into split
patches, in three parts
> +                                       -- according to whether they should be
presented to the
> +                                       -- user: default yes, ask, default no.
> +                                     )
>                -- canonization is needed to undo the effects of splitting
>                -- Typically, the list returned by applySplitter will not
>                -- be in the simplest possible form (since the user will have

Ok, so we will silently accept the first part, ask about the second,
and silently reject the third part. Is the first part useful? On the
other hand, it does no harm being there.

hunk ./src/Darcs/Patch/Split.hs 92
>  -}
>  
>  -- Does not canonize as there is no generic operation to do this.
> -withEditedHead :: Invert p => p C(x y) -> p C(x z) -> FL p C(x y)
> -withEditedHead p res = res :>: invert res :>: p :>: NilFL
> +withEditedHead :: Invert p => p C(x y) -> p C(x z) -> (FL p :> FL p :> FL p)
C(x y)
> +withEditedHead p res = NilFL :> (res :>: NilFL) :> (invert res :>: p :>: NilFL)

We don't add anything silently, we ask the user about their edited
version of the patch, and hide the 'filler' patch from them. Problem:
the hidden 'filler' patch will reappear if they use 'k' in the
interactive selection.


hunk ./src/Darcs/Patch/Split.hs 115
>  noSplitter = Splitter { applySplitter = const Nothing, canonizeSplit = id }
>  
>  
> -doPrimSplit :: Prim C(x y) -> Maybe (B.ByteString, B.ByteString -> Maybe (FL
Prim C(x y)))
> +doPrimSplit :: Prim C(x y) -> Maybe (B.ByteString, B.ByteString -> Maybe ((FL
Prim :> FL Prim :> FL Prim) C(x y)))
>  doPrimSplit (FP fn (Hunk n before after))
>   = Just (B.concat $ intersperse (BC.pack "\n") $ (helptext ++ [sep] ++ before
++ [sep] ++ after ++ [sep]),

ok, will eventually be superseded by the new version of the help text.

hunk ./src/Darcs/Patch/Split.hs 118
> -         \bs -> do let ls = BC.split '\n' bs
> -                   (_, ls') <- breakSep ls
> -                   (before', ls'') <- breakSep ls'
> -                   (after', _) <- breakSep ls''
> -                   return (hunk before before' +>+ hunk before' after' +>+
hunk after' after))
> -    where sep = BC.pack "==="
> +         \bs -> do let intermediates = dropWhile (==before) . revDropWhile
(==after) . -- ignore unchanged blocks
> +                                       tail . -- drop lines before first
separator
> +                                       unfoldr breakSep . -- split on
separator, dropping any lines after last separator
> +                                       BC.split '\n' $ bs
> +                   return (NilFL :> mkHunks before intermediates after))
> +    where sepStr = "==="
> +          sep = BC.pack sepStr
>            helptext = map BC.pack ["Interactive hunk edit:",

ok

hunk ./src/Darcs/Patch/Split.hs 126
> -                                  " - Edit the first set of lines to insert a
new change before the current one.",
> -                                  " - Edit the second set of lines to insert
a new change after the current one."]
> +                                  " The lines between the " ++ sepStr ++ "
separators show the old and new",
> +                                  " states of the hunk. Edit either to
provide an alternative new state.",
> +                                  " You will then be offered the changes
between the alternative new state",
> +                                  " and the old state to confirm."
> +                                 ]
>            hunk b a = canonize (FP fn (Hunk n b a))

This is confusing, but should eventually disappear.

hunk ./src/Darcs/Patch/Split.hs 132
> +          mkHunks b [] a = hunk b a :> NilFL -- no edits made, just offer
original patch
> +          mkHunks b [i] a = hunk b i :> hunk i a -- final "fixup" should
default to no
> +          mkHunks b (i:is) a = case mkHunks i is a of ms :> ns -> (hunk b i
+>+ ms) :> ns
>            breakSep xs = case break (==sep) xs of
>                             (_, []) -> Nothing
>                             (ys, _:zs) -> Just (ys, zs)

ok

hunk ./src/Darcs/Patch/Split.hs 138
> +          revDropWhile p = reverse . dropWhile p . reverse
>  doPrimSplit _ = Nothing
>  
>  -- |Split a primitive hunk patch up

ok

hunk ./src/Darcs/SelectChanges.hs 477
>                  -> do newText <- editText "darcs-patch-edit" text
>                        case parse newText of
>                          Nothing -> repeat_this
> -                        Just ps -> do let tps_new = snd $ patchChoicesTpsSub
(Just (tag tp)) ps
> +                        Just (ys :> ms :> ns)
> +                                -> do let tps_new = snd $ patchChoicesTpsSub
(Just (tag tp)) (ys +>+ ms +>+ ns)
> +                                          tags_new = mapFL tag tps_new
> +                                          yes_tags = take (lengthFL ys) tags_new
> +                                          no_tags = drop (lengthFL ys +
lengthFL ms) tags_new
> +                                          subst_pc = substitute (seal2 (tp
:||: tps_new)) pc
> +                                          new_pc = foldr force_no (foldr
force_yes subst_pc yes_tags) no_tags
>                                        text_select splitter
>                                                    jn whichch critopts (n_max
+ lengthFL tps_new - 1) n

hunk ./src/Darcs/SelectChanges.hs 486
> -                                                  tps_done (tps_new+>+tps_todo')
> -                                                  (substitute (seal2 (tp :||:
tps_new)) pc)
> +                                                  tps_done
(tps_new+>+tps_todo') new_pc
>  
>              's' -> do_next_action "Skipped"  (Noun "change") $ skip_file
>              'f' -> do_next_action "Included" (Noun "change") $ do_file


This puts the new patches in the yes, no or wait bucket of the
patch-choices according to where they are in the FL :> FL :> FL.

----------
status: review-in-progress -> needs-review

__________________________________
Darcs bug tracker <bugs at darcs.net>
<http://bugs.darcs.net/patch146>
__________________________________


More information about the darcs-users mailing list