Re: D3976: grep: add MULTIREV support to --all-files flag
> > I also expect hg grep --all-files -r0+1 foo will show matches from both > rev 0 and 1. > > Suppose there are ten hits in 0 and the same 10 hits in 1, do you mean we > print out all the 20 results, What purpose that would serve? Yes. I think that's the least surprising behavior for --all-files, which is the option to ignore file status. Say a file is modified at both rev 0 and 1, which revisions should be grepped? a. only rev 0 b. rev 0 and rev 1 (because a file is changed at rev 1) c. rev 0 and rev 1 (no matter if a file is changed or not) d. --all-files for rev 0, and --diff for rev 1 (a) is the awkward behavior of the current "hg grep" which we're trying to fix. (b) might sound sensible, but why are unchanged lines in rev 1 displayed again? (c) shows redundant matches, but is consistent. (d) seems a bit tricky, but will be useful. So my proposal was (c). If we take (d), which I think is also good, we'll need to find more appropriate option name than --all-files. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
yuja added a comment. > > I also expect hg grep --all-files -r0+1 foo will show matches from both rev 0 and 1. > > Suppose there are ten hits in 0 and the same 10 hits in 1, do you mean we print out all the 20 results, What purpose that would serve? Yes. I think that's the least surprising behavior for --all-files, which is the option to ignore file status. Say a file is modified at both rev 0 and 1, which revisions should be grepped? a. only rev 0 b. rev 0 and rev 1 (because a file is changed at rev 1) c. rev 0 and rev 1 (no matter if a file is changed or not) d. --all-files for rev 0, and --diff for rev 1 (a) is the awkward behavior of the current "hg grep" which we're trying to fix. (b) might sound sensible, but why are unchanged lines in rev 1 displayed again? (c) shows redundant matches, but is consistent. (d) seems a bit tricky, but will be useful. So my proposal was (c). If we take (d), which I think is also good, we'll need to find more appropriate option name than --all-files. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3976 To: sangeet259, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 added a comment. > I also expect hg grep --all-files -r0+1 foo will show matches from both rev 0 and 1. Suppose there are ten hits in 0 and the same 10 hits in 1, do you mean we print out all the 20 results, What purpose that would serve? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3976 To: sangeet259, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 added a comment. > Keeping all ctx objects might use too much memory. True I will change it to `files[f] = True` REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3976 To: sangeet259, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 added a comment. > I don't think we should omit files that were seen in earlier revisions If I don't skip that would mean the same file in the same state being searched across all the revisions, and getting repetitive and redundant hits. One way I think to circumvent this is by using something like : `if f not in files or f in ctx.files()` and then checking if the new change corresponds to any match. But then, this makes it more similar to `--diff`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3976 To: sangeet259, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
yuja added a comment.
> +# the files dictionary stores all the files that have been looked
at
> +# in the allfiles mode
> +files ={}
I don't think we should omit files that were seen in earlier revisions,
because --all-files is supposed to scan files of any states. I also expect
`hg grep --all-files -r0+1 foo` will show matches from both rev 0 and 1.
What do you think?
> +for f in ctx:
> +if match(f):
> +if f not in files:
> +files[f] = ctx
Keeping all ctx objects might use too much memory.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3976
To: sangeet259, #hg-reviewers
Cc: yuja, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: D3976: grep: add MULTIREV support to --all-files flag
> +# the files dictionary stores all the files that have been looked at
> +# in the allfiles mode
> +files ={}
I don't think we should omit files that were seen in earlier revisions,
because --all-files is supposed to scan files of any states. I also expect
`hg grep --all-files -r0+1 foo` will show matches from both rev 0 and 1.
What do you think?
> +for f in ctx:
> +if match(f):
> +if f not in files:
> +files[f] = ctx
Keeping all ctx objects might use too much memory.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 added a subscriber: yuja. sangeet259 added a comment. @yuja can you review this one REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3976 To: sangeet259, #hg-reviewers Cc: yuja, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 updated this revision to Diff 9664.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3976?vs=9663=9664
REVISION DETAIL
https://phab.mercurial-scm.org/D3976
AFFECTED FILES
mercurial/cmdutil.py
tests/test-grep.t
CHANGE DETAILS
diff --git a/tests/test-grep.t b/tests/test-grep.t
--- a/tests/test-grep.t
+++ b/tests/test-grep.t
@@ -491,3 +491,12 @@
]
$ cd ..
+
+test -rMULTIREV with --all-files
+
+ $ cd sng
+ $ hg rm um
+ $ hg commit -m "deletes um"
+ $ hg grep -r "0:2" "unmod" --all-files
+ um:0:unmod
+ $ cd ..
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1889,9 +1889,6 @@
revs = _walkrevs(repo, opts)
if not revs:
return []
-if allfiles and len(revs) > 1:
-raise error.Abort(_("multiple revisions not supported with "
-"--all-files"))
wanted = set()
slowpath = match.anypats() or (not match.always() and opts.get('removed'))
fncache = {}
@@ -1983,6 +1980,11 @@
it = iter(revs)
stopiteration = False
+
+# the files dictionary stores all the files that have been looked at
+# in the allfiles mode
+files = {}
+
for windowsize in increasingwindows():
nrevs = []
for i in xrange(windowsize):
@@ -1998,12 +2000,15 @@
if not fns:
def fns_generator():
if allfiles:
-fiter = iter(ctx)
+for f in ctx:
+if match(f):
+if f not in files:
+files[f] = ctx
+yield f
else:
-fiter = ctx.files()
-for f in fiter:
-if match(f):
-yield f
+for f in ctx.files():
+if match(f):
+yield f
fns = fns_generator()
prepare(ctx, fns)
for rev in nrevs:
To: sangeet259, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 updated this revision to Diff 9663.
sangeet259 edited the summary of this revision.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D3976?vs=9657=9663
REVISION DETAIL
https://phab.mercurial-scm.org/D3976
AFFECTED FILES
mercurial/cmdutil.py
tests/test-grep.t
CHANGE DETAILS
diff --git a/tests/test-grep.t b/tests/test-grep.t
--- a/tests/test-grep.t
+++ b/tests/test-grep.t
@@ -491,3 +491,12 @@
]
$ cd ..
+
+test -rMULTIREV with --all-files
+
+ $ cd sng
+ $ hg rm um
+ $ hg commit -m "deletes um"
+ $ hg grep -r "0:2" "unmod" --all-files
+ um:0:unmod
+ $ cd ..
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1889,9 +1889,7 @@
revs = _walkrevs(repo, opts)
if not revs:
return []
-if allfiles and len(revs) > 1:
-raise error.Abort(_("multiple revisions not supported with "
-"--all-files"))
+
wanted = set()
slowpath = match.anypats() or (not match.always() and opts.get('removed'))
fncache = {}
@@ -1983,6 +1981,11 @@
it = iter(revs)
stopiteration = False
+
+# the files dictionary stores all the files that have been looked at
+# in the allfiles mode
+files ={}
+
for windowsize in increasingwindows():
nrevs = []
for i in xrange(windowsize):
@@ -1998,12 +2001,15 @@
if not fns:
def fns_generator():
if allfiles:
-fiter = iter(ctx)
+for f in ctx:
+if match(f):
+if f not in files:
+files[f] = ctx
+yield f
else:
-fiter = ctx.files()
-for f in fiter:
-if match(f):
-yield f
+for f in ctx.files():
+if match(f):
+yield f
fns = fns_generator()
prepare(ctx, fns)
for rev in nrevs:
To: sangeet259, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D3976: grep: add MULTIREV support to --all-files flag
sangeet259 created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3976
AFFECTED FILES
mercurial/cmdutil.py
tests/test-grep.t
CHANGE DETAILS
diff --git a/tests/test-grep.t b/tests/test-grep.t
--- a/tests/test-grep.t
+++ b/tests/test-grep.t
@@ -491,3 +491,12 @@
]
$ cd ..
+
+test -rMULTIREV with --all-files
+
+ $ cd sng
+ $ hg rm um
+ $ hg commit -m "deletes um"
+ $ hg grep -r "0:2" "unmod" --all-files
+ um:0:unmod
+ $ cd ..
diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1983,6 +1983,11 @@
it = iter(revs)
stopiteration = False
+
+# the files dictionary stores all the files that have been looked at
+# in the allfiles mode
+files ={}
+
for windowsize in increasingwindows():
nrevs = []
for i in xrange(windowsize):
@@ -1998,12 +2003,15 @@
if not fns:
def fns_generator():
if allfiles:
-fiter = iter(ctx)
+for f in ctx:
+if match(f):
+if f not in files:
+files[f] = ctx
+yield f
else:
-fiter = ctx.files()
-for f in fiter:
-if match(f):
-yield f
+for f in ctx.files():
+if match(f):
+yield f
fns = fns_generator()
prepare(ctx, fns)
for rev in nrevs:
To: sangeet259, #hg-reviewers
Cc: mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
