D1336: remove: print message for each file in verbose mode only while using `-A`
This revision was automatically updated to reflect the committed changes. Closed by commit rHG7a58608281dd: remove: print message for each file in verbose mode only while using `-A` (BC) (authored by pavanpc, committed by ). CHANGED PRIOR TO COMMIT https://phab.mercurial-scm.org/D1336?vs=3621=3723#toc REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1336?vs=3621=3723 REVISION DETAIL https://phab.mercurial-scm.org/D1336 AFFECTED FILES mercurial/cmdutil.py tests/test-remove.t CHANGE DETAILS diff --git a/tests/test-remove.t b/tests/test-remove.t --- a/tests/test-remove.t +++ b/tests/test-remove.t @@ -189,9 +189,9 @@ \r (no-eol) (esc) 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -21 state clean, options -A +21 state clean, options -Av - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -205,10 +205,10 @@ ./foo 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -22 state modified, options -A +22 state modified, options -Av $ echo b >> foo - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -357,9 +357,32 @@ \r (no-eol) (esc) 2 files updated, 0 files merged, 0 files removed, 0 files unresolved -dir, options -A +dir, options -Av $ rm test/bar + $ remove -Av test + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + skipping [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + removing test/bar (glob) + not removing test/foo: file still exists (glob) + exit code: 1 + R test/bar + ./foo + ./test/foo + \r (no-eol) (esc) + updating [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + +dir, options -A + $ rm test/bar $ remove -A test \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) @@ -371,7 +394,26 @@ deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) removing test/bar (glob) - not removing test/foo: file still exists (glob) + exit code: 1 + R test/bar + ./foo + ./test/foo + \r (no-eol) (esc) + updating [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + +without any files/dirs, options -A + $ rm test/bar + $ remove -A + \r (no-eol) (esc) + skipping [=> ] 1/2\r (no-eol) (esc) + skipping [===>] 2/2\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + removing test/bar (glob) exit code: 1 R test/bar ./foo diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2982,8 +2982,9 @@ for f in remaining: count += 1 ui.progress(_('skipping'), count, total=total, unit=_('files')) -warnings.append(_('not removing %s: file still exists\n') -% m.rel(f)) +if ui.verbose or (f in files): +warnings.append(_('not removing %s: file still exists\n') +% m.rel(f)) ret = 1 ui.progress(_('skipping'), None) else: To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
yuja accepted this revision. yuja added a comment. This revision is now accepted and ready to land. Queued, thanks. I've added "(BC)" to the commit message. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mitrandir added a comment. @durin42: I think it should be as good as possible BC-wise. What do you think? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc added a comment. @yuja Thank you for your input. Now, warnings are suppressed for the directory case. Also, I have added test cases for the above cases. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc updated this revision to Diff 3621. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1336?vs=3435=3621 REVISION DETAIL https://phab.mercurial-scm.org/D1336 AFFECTED FILES mercurial/cmdutil.py tests/test-remove.t CHANGE DETAILS diff --git a/tests/test-remove.t b/tests/test-remove.t --- a/tests/test-remove.t +++ b/tests/test-remove.t @@ -189,9 +189,9 @@ \r (no-eol) (esc) 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -21 state clean, options -A +21 state clean, options -Av - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -205,10 +205,10 @@ ./foo 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -22 state modified, options -A +22 state modified, options -Av $ echo b >> foo - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -357,9 +357,32 @@ \r (no-eol) (esc) 2 files updated, 0 files merged, 0 files removed, 0 files unresolved -dir, options -A +dir, options -Av $ rm test/bar + $ remove -Av test + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + skipping [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + removing test/bar (glob) + not removing test/foo: file still exists (glob) + exit code: 1 + R test/bar + ./foo + ./test/foo + \r (no-eol) (esc) + updating [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + +dir, options -A + $ rm test/bar $ remove -A test \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) @@ -371,7 +394,26 @@ deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) removing test/bar (glob) - not removing test/foo: file still exists (glob) + exit code: 1 + R test/bar + ./foo + ./test/foo + \r (no-eol) (esc) + updating [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + 1 files updated, 0 files merged, 0 files removed, 0 files unresolved + +without any files/dirs, options -A + $ rm test/bar + $ remove -A + \r (no-eol) (esc) + skipping [=> ] 1/2\r (no-eol) (esc) + skipping [===>] 2/2\r (no-eol) (esc) + \r (no-eol) (esc) + \r (no-eol) (esc) + deleting [===>] 1/1\r (no-eol) (esc) + \r (no-eol) (esc) + removing test/bar (glob) exit code: 1 R test/bar ./foo diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2975,8 +2975,9 @@ for f in remaining: count += 1 ui.progress(_('skipping'), count, total=total, unit=_('files')) -warnings.append(_('not removing %s: file still exists\n') -% m.rel(f)) +if ui.verbose or (f in files): +warnings.append(_('not removing %s: file still exists\n') +% m.rel(f)) ret = 1 ui.progress(_('skipping'), None) else: To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
yuja added a comment. In https://phab.mercurial-scm.org/D1336#23872, @pavanpc wrote: > 4. hg rm -A - What should be the behavior here? What if the directory has thousands of files . May be we can suppress warnings based on number of files in the warnings list, like if len(warnings_list) > 25 suppress warnings ? or always suppress warnings for this case I think "always suppress" is the right thing to do because I would run `hg rm -A ` to record deleted files under the directory. In matcher, I think only `m.files()` needs to be warned. > -I PATTERN or -X PATTERN I see these patterns are not "explicit" files, so won't be warned. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mharbison72 added a comment. I guess the other case is when -I PATTERN or -X PATTERN are used instead of file names. These will be inexact matches like when a directory is specified. I don’t like the dual behavior controlled by how many files are in the directory, because the it will seem inconsistent. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mitrandir added a comment. I think that the cases mentioned by you are right :) In case of directory I'd prefer so suppress warns. What do you think @yuja ? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc added a comment. @yuja @mitrandir I can think of below cases. Could you please check and let me know your thoughts on these? 1. hg rm -A- suppress warnings 2. hg rm -Av - do not suppress warnings (could be helpful for small repos) 3. hg rm -A- do not suppress warnings 4. hg rm -A - What should be the behavior here? What if the directory has thousands of files . May be we can suppress warnings based on number of files in the warnings list, like if len(warnings_list) > 25 supprress warnings ? or always suppress warnings for this case 5. hg rm -Av - do not suppress warnings REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
yuja requested changes to this revision. yuja added a comment. This revision now requires changes to proceed. In https://phab.mercurial-scm.org/D1336#23145, @mitrandir wrote: > @durin42: I'd say that listing *all the files in the repo* when you run `hg rm -A` should be considered a bug. Agreed. Only explicitly-specified files should be warned by default. And suppressing all warnings seems wrong as Augie said and is BC. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan, yuja Cc: yuja, durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mitrandir added a comment. @durin42: I'd say that listing *all the files in the repo* when you run `hg rm -A` should be considered a bug. This can't possibly be the intended behavior. Also: running `hg rm -A` without a `file` argument is not even covered by any testcase. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan Cc: durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mharbison72 added a comment. If this can't be taken as-is, what about something in tweakdefaults to hide this by default? I just assumed that --quiet already worked, and the problem was that this throws away the other more useful warnings in the non --after case. (Or people don't remember to use --quiet before running it.) FWIW, I watched someone run `hg rm -A` on a repo with 67K files on Friday, and that made me much more sympathetic. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan Cc: durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
durin42 added a comment. I think this probably violates our BC policy. We could probably stand to add a `--quiet` for suppressing this output, I suppose. I'll leave this open for other reviewers to mull over, and if it's still open in a week without comment I'll probably pass final judgement. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan Cc: durin42, lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
lothiraldan accepted this revision. lothiraldan added a comment. In https://phab.mercurial-scm.org/D1336#22841, @pavanpc wrote: > @lothiraldan I modified a test case for hg rm -A case where we get the message 'not removing : file still exists' . For other cases, test cases already exist with 'hg rm -A' and 'hg rm --after' without verbose mode. Thx, LGTM REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers, lothiraldan Cc: lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc added a comment. @lothiraldan I modified a test case for hg rm -A case where we get the message 'not removing : file still exists' . For other cases, test cases already exist with 'hg rm -A' and 'hg rm --after' without verbose mode. INLINE COMMENTS > mharbison72 wrote in cmdutil.py:2982 > 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? Moved the 'remaining ' outside the conditional. Making the ui.verbose check only while adding it to the warnings list REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers Cc: lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc updated this revision to Diff 3435. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1336?vs=3424=3435 REVISION DETAIL https://phab.mercurial-scm.org/D1336 AFFECTED FILES mercurial/cmdutil.py tests/test-remove.t CHANGE DETAILS diff --git a/tests/test-remove.t b/tests/test-remove.t --- a/tests/test-remove.t +++ b/tests/test-remove.t @@ -179,7 +179,6 @@ \r (no-eol) (esc) skipping [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) - not removing bar: file still exists exit code: 1 A bar ./bar @@ -189,9 +188,9 @@ \r (no-eol) (esc) 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -21 state clean, options -A +21 state clean, options -Av - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -205,10 +204,10 @@ ./foo 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -22 state modified, options -A +22 state modified, options -Av $ echo b >> foo - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -357,10 +356,10 @@ \r (no-eol) (esc) 2 files updated, 0 files merged, 0 files removed, 0 files unresolved -dir, options -A +dir, options -Av $ rm test/bar - $ remove -A test + $ remove -Av test \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2975,8 +2975,9 @@ for f in remaining: count += 1 ui.progress(_('skipping'), count, total=total, unit=_('files')) -warnings.append(_('not removing %s: file still exists\n') -% m.rel(f)) +if ui.verbose: +warnings.append(_('not removing %s: file still exists\n') +% m.rel(f)) ret = 1 ui.progress(_('skipping'), None) else: To: pavanpc, #hg-reviewers Cc: lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
lothiraldan added a comment. Could we have a test for `hg rm -A` without the verbose mode? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers Cc: lothiraldan, mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
mharbison72 added a comment. Looks good to me, thanks. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D1336 To: pavanpc, #hg-reviewers Cc: mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc updated this revision to Diff 3424. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1336?vs=3423=3424 REVISION DETAIL https://phab.mercurial-scm.org/D1336 AFFECTED FILES mercurial/cmdutil.py tests/test-remove.t CHANGE DETAILS diff --git a/tests/test-remove.t b/tests/test-remove.t --- a/tests/test-remove.t +++ b/tests/test-remove.t @@ -168,11 +168,11 @@ \r (no-eol) (esc) 1 files updated, 0 files merged, 0 files removed, 0 files unresolved -20 state added, options -A +20 state added, options -Av $ echo b > bar $ hg add bar - $ remove -A bar + $ remove -Av bar \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -189,9 +189,9 @@ \r (no-eol) (esc) 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -21 state clean, options -A +21 state clean, options -Av - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -205,10 +205,10 @@ ./foo 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -22 state modified, options -A +22 state modified, options -Av $ echo b >> foo - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -357,10 +357,10 @@ \r (no-eol) (esc) 2 files updated, 0 files merged, 0 files removed, 0 files unresolved -dir, options -A +dir, options -Av $ rm test/bar - $ remove -A test + $ remove -Av test \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2975,8 +2975,9 @@ for f in remaining: count += 1 ui.progress(_('skipping'), count, total=total, unit=_('files')) -warnings.append(_('not removing %s: file still exists\n') -% m.rel(f)) +if ui.verbose: +warnings.append(_('not removing %s: file still exists\n') +% m.rel(f)) ret = 1 ui.progress(_('skipping'), None) else: To: pavanpc, #hg-reviewers Cc: mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
pavanpc updated this revision to Diff 3423. pavanpc marked an inline comment as done. REPOSITORY rHG Mercurial CHANGES SINCE LAST UPDATE https://phab.mercurial-scm.org/D1336?vs=3391=3423 REVISION DETAIL https://phab.mercurial-scm.org/D1336 AFFECTED FILES mercurial/cmdutil.py tests/test-remove.t CHANGE DETAILS diff --git a/tests/test-remove.t b/tests/test-remove.t --- a/tests/test-remove.t +++ b/tests/test-remove.t @@ -168,11 +168,11 @@ \r (no-eol) (esc) 1 files updated, 0 files merged, 0 files removed, 0 files unresolved -20 state added, options -A +20 state added, options -Av $ echo b > bar $ hg add bar - $ remove -A bar + $ remove -Av bar \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -189,9 +189,9 @@ \r (no-eol) (esc) 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -21 state clean, options -A +21 state clean, options -Av - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -205,10 +205,10 @@ ./foo 0 files updated, 0 files merged, 0 files removed, 0 files unresolved -22 state modified, options -A +22 state modified, options -Av $ echo b >> foo - $ remove -A foo + $ remove -Av foo \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -357,10 +357,10 @@ \r (no-eol) (esc) 2 files updated, 0 files merged, 0 files removed, 0 files unresolved -dir, options -A +dir, options -Av $ rm test/bar - $ remove -A test + $ remove -Av test \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) @@ -448,7 +448,7 @@ [1] $ hg add d1/a $ rm d1/a - $ hg rm --after d1 + $ hg rm --after d1 \r (no-eol) (esc) deleting [===>] 1/1\r (no-eol) (esc) \r (no-eol) (esc) diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py --- a/mercurial/cmdutil.py +++ b/mercurial/cmdutil.py @@ -2975,8 +2975,9 @@ for f in remaining: count += 1 ui.progress(_('skipping'), count, total=total, unit=_('files')) -warnings.append(_('not removing %s: file still exists\n') -% m.rel(f)) +if ui.verbose: +warnings.append(_('not removing %s: file still exists\n') +% m.rel(f)) ret = 1 ui.progress(_('skipping'), None) else: To: pavanpc, #hg-reviewers Cc: mitrandir, mharbison72, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D1336: remove: print message for each file in verbose mode only while using `-A`
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 ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel