D1336: remove: print message for each file in verbose mode only while using `-A`

mharbison72 (Matt Harbison) phabricator at mercurial-scm.org
Fri Nov 10 20:12:08 EST 2017


mharbison72 added a comment.


  @pavanpc Looks pretty good- just a few things that popped up in the last revision.

INLINE COMMENTS

> cmdutil.py:2982
> +            ui.progress(_('skipping'), None)
> +        ret = 1
>      else:

This will need to be protected by seeing if there are any files in modified + added + clean, like it was before.  Otherwise, using -A will always return non zero, even if it succeeded without warning cases.  Maybe hoist the 'remaining = ...' line out of the conditional?

> test-remove.t:236
>                                                                \r (no-eol) (esc)
> -  exit code: 0
> +  exit code: 1
>    R foo

Here's the evidence of the exit code problem.

> test-remove.t:451
>    $ rm d1/a
> -  $ hg rm --after d1
> +  $ hg rm --after   d1
>    \r (no-eol) (esc)

Please avoid unnecessary changes like this.  It may seem trivial, but if everyone did it, it would make annotating more of a chore.

> test-remove.t:458
>                                                                \r (no-eol) (esc)
> -  removing d1/a (glob)
> +  removing d1/a
> +  [1]

I don't see any reason for the glob to go away.  I suspect it may have been due to the exit code changing in the next line.  In general, be careful about not dropping these.  The test runner generally doesn't stick them in when run on anything but Windows, and not having them causes test failures on Windows.  It's a work around for Windows printing 'removing d1\a' on this same line.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D1336

To: pavanpc, #hg-reviewers
Cc: mitrandir, mharbison72, mercurial-devel


More information about the Mercurial-devel mailing list