Hi,

this is a draft implementation of revert -o/-O. This should work reasonably
well, with two caveats (and both are probably good reasons to defer this
patch):

1) The bundle is written out with current repository context: this means that
obliterating local patches will render it unapplicable. (There would be
identical problem with naive stash implementation... stash would probably need
to add logic to obliterate and amend-record (and eventually rebase) which would
commute down all stashed patches. Either that, or just refuse to work if
something is stashed. How git-like...)

2) Applying the patch to working (and working only) is not supported by darcs
apply. Having something like darcs apply --working would help, presumably. The
current hint about apply + unrecord/amend may be workable, but not great.

The problem 1) may be an argument for real stash, although personally I'd much
prefer in-repo branching and stashing based on that. Also, with existing darcs
code, doing "real" stash may be pretty complex.

Yours,
   Petr.

1 patch for repository http://darcs.net/:

Thu Jul 15 03:55:07 CEST 2010  Petr Rockai <me@mornfall.net>
  * DRAFT Resolve issue1891: implement darcs revert -o/-O.

Attachments

• draft-resolve-issue1891_-implement-darcs-revert-_o__o_.dpatch
• unnamed

Just amended to follow patch305.

1 patch for repository http://darcs.net/:

Thu Jul 15 11:37:28 CEST 2010  Petr Rockai <me@mornfall.net>
  * DRAFT Resolve issue1891: implement darcs revert -o/-O.

Attachments

• draft-resolve-issue1891_-implement-darcs-revert-_o__o_.dpatch
• unnamed

Not a review, just a question.

DRAFT Resolve issue1891: implement darcs revert -o/-O.
------------------------------------------------------
>  revertCmd :: [DarcsFlag] -> [String] -> IO ()
>  revertCmd opts args = withRepoLock opts $- \repository -> do
> hunk ./src/Darcs/Commands/Revert.lhs 97
>    changes <- unrecordedChanges opts repository files
>    let pre_changed_files = applyToFilepaths (invert changes) (map toFilePath files)
>    rec <- readRecorded repository
> +  let unrevert ps | Just out <- getOutput opts "stash.dpatch" =
> +        do allpatches <- newset2RL `fmap` readRepo repository -- TODO minimize the context here?
> +           date <- getDate opts
> +           author <- getAuthor opts
> +           info <- (patchinfo date "stash" author [])
> +           savetoBundle opts allpatches $ n2pia (infopatch info $ fromPrims ps) :>: NilFL
> +           putStrLn $ "To unrevert this patch, use darcs apply " ++ show' out ++ ",\n"
> +                       ++ "followed by darcs unrecord or darcs amend --edit."
> +                       | otherwise = writeUnrevert repository ps rec NilFL
> +      show' = drop 1 . init . show

What exactly happens if the stash file already exists?

I seem to remember that darcs send -o just clobbers it, but in the case
of stashing, would that be desirable?  (Particularly if the user just
does a revert -O stash-dir)

> +
>    Sealed touching_changes <- return (chooseTouching pre_changed_files changes)
>    (case touching_changes of
>      NilFL -> putStrLn "There are no changes to revert!"
> hunk ./src/Darcs/Commands/Revert.lhs 128
>                   addToPending repository $ invert p
>                   when (Debug `elem` opts) $ putStrLn "About to write the unrevert file."
>                   case commute (norevert:>p) of
> -                   Just (p':>_) -> writeUnrevert repository p' rec NilFL
> +                   Just (p':>_) -> unrevert p'
> -                   Nothing -> writeUnrevert repository (norevert+>+p) rec NilFL
> +                   Nothing -> unrevert (norevert+>+p)

What's happened here?

-- 
Eric Kow <http://www.nltg.brighton.ac.uk/home/Eric.Kow>
For a faster response, please try +44 (0)1273 64 2905.

Eric Kow <kowey@darcs.net> writes:

> What exactly happens if the stash file already exists?
It is clobbered...

> I seem to remember that darcs send -o just clobbers it, but in the case
> of stashing, would that be desirable?  (Particularly if the user just
> does a revert -O stash-dir)
Yes, I think getOutput or something should be modified to give a unique
name (in all cases, not just for stash). Clobbering is not very nice.

>> -                   Just (p':>_) -> writeUnrevert repository p' rec NilFL
>> +                   Just (p':>_) -> unrevert p'
>> -                   Nothing -> writeUnrevert repository (norevert+>+p) rec NilFL
>> +                   Nothing -> unrevert (norevert+>+p)
>
> What's happened here?
Just use the new "unrevert" helper that decides whether to write to a
bundle or to write _darcs/patches/unrevert. Maybe it needs a better
name.

Yours,
   Petr.

Sweep this under the rug for now. I'll re-do this sometime post-adventure 
or something.