[PATCH 1 of 5] revert: simplify handling of `added` files

[Augie Fackler]

On Fri, Aug 15, 2014 at 09:48:43AM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david at fb.com>
> # Date 1407004344 25200
> #      Sat Aug 02 11:32:24 2014 -0700
> # Node ID 2eb66256ddb71face40ed113a98fd465aa9579da
> # Parent  1a4de7e50e27a568b77854e0bbfc894d48a726b0
> revert: simplify handling of `added` files

Series looks fine, but I have conflicts in cmdutil - can you resend?

>
> They are multiple possible cases for added files. But its all handled by magic
> much lower in the stack. We document them, simplify the codes and move on.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2398,23 +2398,39 @@ def revert(ui, repo, ctx, parents, *pats
>          else:
>              changes = repo.status(node1=parent, match=m)
>              dsmodified = set(changes[0])
>              dsadded    = set(changes[1])
>              dsremoved  = set(changes[2])
> -            dsadded |= _deletedadded
>
>              # only take in account for remove that between wc and target.
>              clean |= dsremoved - removed
>              dsremoved &= removed
>              # distinct between dirstate remove and other
>              removed -= dsremoved
>
> +
>              # tell newly modified appart.
>              dsmodified &= modified
>              dsmodified |= modified & dsadded # dirstate added may needs backup
>              modified -= dsmodified
>
> +            # there is three categories of added files
> +            #
> +            # 1. addition, that just happened in the dirstate
> +            #    (should be forgetten)
> +            # 2. file is added since target revision and have local changes
> +            #    (should be backed up and removed)
> +            # 3. file is added since target revision and is clean
> +            #    (should be removed)
> +            #
> +            # However we do not need to split them yet. The current revert code
> +            # will automatically recognise (1) when performing operation. And
> +            # the backup system is currently unabled to handle (2).
> +            #
> +            # So we just put them all in the same group
> +            dsadded = added
> +
>          # if f is a rename, update `names` to also revert the source
>          cwd = repo.getcwd()
>          for f in dsadded:
>              src = repo.dirstate.copied(f)
>              if src and src not in names and repo.dirstate[src] == 'r':
> @@ -2428,12 +2444,10 @@ def revert(ui, repo, ctx, parents, *pats
>                  return _('forgetting %s\n')
>              return _('removing %s\n')
>
>          missingmodified = dsmodified - smf
>          dsmodified -= missingmodified
> -        missingadded = dsadded - smf
> -        dsadded -= missingadded
>
>          # action to be actually performed by revert
>          # (<list of file>, message>) tuple
>          actions = {'revert': ([], _('reverting %s\n')),
>                     'add': ([], _('adding %s\n')),
> @@ -2446,12 +2460,11 @@ def revert(ui, repo, ctx, parents, *pats
>              #   action
>              #   make backup
>              (modified,         (actions['revert'],   False)),
>              (dsmodified,       (actions['revert'],   True)),
>              (missingmodified,  (actions['remove'],   True)),
> -            (dsadded,          (actions['revert'],   True)),
> -            (missingadded,     (actions['remove'],   False)),
> +            (dsadded,          (actions['remove'],   True)),
>              (removed,          (actions['add'],      True)),
>              (dsremoved,        (actions['undelete'], True)),
>              (clean,            (None,                False)),
>              )
>
> diff --git a/tests/test-revert.t b/tests/test-revert.t
> --- a/tests/test-revert.t
> +++ b/tests/test-revert.t
> @@ -906,16 +906,10 @@ Test revert --all to "base" content
>    $ cp -r revert-ref revert-base-all
>    $ cd revert-base-all
>
>  check revert output
>
> -Misbehavior:
> -
> -- report "reverting" when file needs no changes
> -|
> -| - reverting removed_revert
> -
>    $ hg revert --all --rev 'desc(base)'
>    removing added_clean
>    removing added_deleted
>    removing added_wc
>    reverting clean_deleted
> @@ -934,11 +928,10 @@ Misbehavior:
>    undeleting modified_untracked-wc
>    reverting modified_wc
>    adding removed_clean
>    reverting removed_deleted
>    adding removed_removed
> -  reverting removed_revert
>    adding removed_untracked-clean
>    adding removed_untracked-revert
>    adding removed_untracked-wc
>    reverting removed_wc
>
> @@ -1096,16 +1089,10 @@ Test revert to "base" content with expli
>    $ cd revert-base-explicit
>
>  revert all files individually and check the output
>  (output is expected to be different than in the --all case)
>
> -Misbehavior:
> -
> -- fails to report no change to revert for
> -|
> -| - removed_revert
> -
>    $ for file in `python ../gen-revert-cases.py filelist`; do
>    >   echo '### revert for:' $file;
>    >   hg revert $file --rev 'desc(base)';
>    >   echo
>    > done
> @@ -1192,10 +1179,11 @@ Misbehavior:
>    ### revert for: removed_deleted
>
>    ### revert for: removed_removed
>
>    ### revert for: removed_revert
> +  no changes needed to removed_revert
>
>    ### revert for: removed_untracked-clean
>
>    ### revert for: removed_untracked-revert
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel