[PATCH 2 of 3 v2] remove: queue warnings until after status messages (issue5140) (API)

Martijn Pieters mj at zopatista.com
Thu Mar 24 18:22:06 EDT 2016


On 21 March 2016 at 13:21, timeless <timeless at mozdev.org> wrote:
> # HG changeset patch
> # User timeless <timeless at mozdev.org>
> # Date 1458238776 0
> #      Thu Mar 17 18:19:36 2016 +0000
> # Node ID 3a5be722f31727d64496d00dd5dd11e02bc7bc72
> # Parent  9923fa5a10511ddd1207f9019e8d50b29f49aaca
> remove: queue warnings until after status messages (issue5140) (API)
>
> Before this change, warnings were interspersed with (and easily drowned out by)
> status messages.
>
> API:
> abstractsubrepo.removefiles has an extra argument warnings,
> into which callees should append their warnings.
>   Note: Callees should not assume that there will be items in the list,
>   today, I'm lazily including any other subrepos warnings, but
>   that may change.
>
> cmdutil.remove has an extra optional argument warnings,
> into which it will place warnings.
> If warnings is omitted, warnings will be reported via ui.warn()
> as before this change (albeit, after any status messages).
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -2393,7 +2393,7 @@
>
>      return ret
>
> -def remove(ui, repo, m, prefix, after, force, subrepos):
> +def remove(ui, repo, m, prefix, after, force, subrepos, warnings=None):
>      join = lambda f: os.path.join(prefix, f)
>      ret = 0
>      s = repo.status(match=m, clean=True)
> @@ -2401,6 +2401,12 @@
>
>      wctx = repo[None]
>
> +    if warnings is None:
> +        warnings = []
> +        warn = True
> +    else:
> +        warn = False
> +

Why the separate `warn` flag? Why not use `if warnings` instead?

>      for subpath in sorted(wctx.substate):
>          def matchessubrepo(matcher, subpath):
>              if matcher.exact(subpath):
> @@ -2414,10 +2420,11 @@
>              sub = wctx.sub(subpath)
>              try:
>                  submatch = matchmod.subdirmatcher(subpath, m)
> -                if sub.removefiles(submatch, prefix, after, force, subrepos):
> +                if sub.removefiles(submatch, prefix, after, force, subrepos,
> +                                   warnings):
>                      ret = 1
>              except error.LookupError:
> -                ui.status(_("skipping missing subrepository: %s\n")
> +                warnings.append(_("skipping missing subrepository: %s\n")
>                                 % join(subpath))
>
>      # warn about failure to delete explicit files/dirs
> @@ -2435,10 +2442,10 @@
>
>          if repo.wvfs.exists(f):
>              if repo.wvfs.isdir(f):
> -                ui.warn(_('not removing %s: no tracked files\n')
> +                warnings.append(_('not removing %s: no tracked files\n')
>                          % m.rel(f))
>              else:
> -                ui.warn(_('not removing %s: file is untracked\n')
> +                warnings.append(_('not removing %s: file is untracked\n')
>                          % m.rel(f))
>          # missing files will generate a warning elsewhere
>          ret = 1
> @@ -2448,16 +2455,16 @@
>      elif after:
>          list = deleted
>          for f in modified + added + clean:
> -            ui.warn(_('not removing %s: file still exists\n') % m.rel(f))
> +            warnings.append(_('not removing %s: file still exists\n') % m.rel(f))
>              ret = 1
>      else:
>          list = deleted + clean
>          for f in modified:
> -            ui.warn(_('not removing %s: file is modified (use -f'
> +            warnings.append(_('not removing %s: file is modified (use -f'
>                        ' to force removal)\n') % m.rel(f))
>              ret = 1
>          for f in added:
> -            ui.warn(_('not removing %s: file has been marked for add'
> +            warnings.append(_('not removing %s: file has been marked for add'
>                        ' (use forget to undo)\n') % m.rel(f))
>              ret = 1
>
> @@ -2473,6 +2480,10 @@
>                  util.unlinkpath(repo.wjoin(f), ignoremissing=True)
>          repo[None].forget(list)
>
> +    if warn:
> +        for warning in warnings:
> +            ui.warn(warning)
> +
>      return ret
>
>  def cat(ui, repo, ctx, matcher, prefix, **opts):
> diff --git a/mercurial/subrepo.py b/mercurial/subrepo.py
> --- a/mercurial/subrepo.py
> +++ b/mercurial/subrepo.py
> @@ -575,11 +575,13 @@
>      def forget(self, match, prefix):
>          return ([], [])
>
> -    def removefiles(self, matcher, prefix, after, force, subrepos):
> +    def removefiles(self, matcher, prefix, after, force, subrepos, warnings):
>          """remove the matched files from the subrepository and the filesystem,
>          possibly by force and/or after the file has been removed from the
>          filesystem.  Return 0 on success, 1 on any warning.
>          """
> +        warnings.append(_("warning: removefiles not implemented (%s)")
> +                        % self._path)
>          return 1
>
>      def revert(self, substate, *pats, **opts):
> @@ -991,7 +993,7 @@
>                                self.wvfs.reljoin(prefix, self._path), True)
>
>      @annotatesubrepoerror
> -    def removefiles(self, matcher, prefix, after, force, subrepos):
> +    def removefiles(self, matcher, prefix, after, force, subrepos, warnings):
>          return cmdutil.remove(self.ui, self._repo, matcher,
>                                self.wvfs.reljoin(prefix, self._path),
>                                after, force, subrepos)
> diff --git a/tests/test-remove.t b/tests/test-remove.t
> --- a/tests/test-remove.t
> +++ b/tests/test-remove.t
> @@ -277,8 +277,8 @@
>
>    $ rm test/bar
>    $ remove -A test
> +  removing test/bar (glob)
>    not removing test/foo: file still exists (glob)
> -  removing test/bar (glob)
>    exit code: 1
>    R test/bar
>    ./foo
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



-- 
Martijn Pieters


More information about the Mercurial-devel mailing list