Submitter | phabricator |
---|---|
Date | May 6, 2018, 1:13 a.m. |
Message ID | <differential-rev-PHID-DREV-mxwcohnn3gpi574zm7nt-req@phab.mercurial-scm.org> |
Download | mbox | patch |
Permalink | /patch/31265/ |
State | Superseded |
Headers | show |
Comments
Can you add test for failure cases? such as * `str|commonprefix` * `int|commonprefix` * nothing common (e.g. `["/foo", "bar"]`) * empty list * paths normalized by `os.path.normcase()` `os.path.commonprefix()` isn't what we want, but it might provide some hints.
yuja added a comment. Can you add test for failure cases? such as - `str|commonprefix` - `int|commonprefix` - nothing common (e.g. `["/foo", "bar"]`) - empty list - paths normalized by `os.path.normcase()` `os.path.commonprefix()` isn't what we want, but it might provide some hints. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: yuja, mercurial-devel
"`os.path.commonprefix()` isn't what we want, but it might provide some hints." Perhaps `os.path.commonprefix()` could be used with conditional fixup by `os.path.dirname()`? ``` prefix = os.path.commonprefix(files) if prefix isn't a valid path: prefix = os.path.dirname(prefix) ``` This way, we can remove `normpath()` which has a side effect `s|/|\\|g` on Windows.
yuja added a comment. "`os.path.commonprefix()` isn't what we want, but it might provide some hints." Perhaps `os.path.commonprefix()` could be used with conditional fixup by `os.path.dirname()`? prefix = os.path.commonprefix(files) if prefix isn't a valid path: prefix = os.path.dirname(prefix) This way, we can remove `normpath()` which has a side effect `s|/|\\|g` on Windows. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: yuja, mercurial-devel
joerg.sonnenberger added a comment. {str|commonprefix} is not really interesting since it is naturally an iterable of text. The others are covered, the routine tries to optimize a couple of common cases as well now. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: yuja, mercurial-devel
martinvonz added a comment. Do you think we should call it `commonpath()` or `commondir()` instead in case we want `commonprefix()` to work for any string in the future (and not care about path separators)? REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: martinvonz, yuja, mercurial-devel
joerg.sonnenberger added a comment.
In https://phab.mercurial-scm.org/D3439#58405, @martinvonz wrote:
> Do you think we should call it `commonpath()` or `commondir()` instead in case we want `commonprefix()` to work for any string in the future (and not care about path separators)?
`commondir()` would be fine as well. `commonpath()` doesn't feel better. I'm ambivalent about either name.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D3439
To: joerg.sonnenberger, #hg-reviewers
Cc: martinvonz, yuja, mercurial-devel
martinvonz added a comment. In https://phab.mercurial-scm.org/D3439#58410, @joerg.sonnenberger wrote: > In https://phab.mercurial-scm.org/D3439#58405, @martinvonz wrote: > > > Do you think we should call it `commonpath()` or `commondir()` instead in case we want `commonprefix()` to work for any string in the future (and not care about path separators)? > > > `commondir()` would be fine as well. `commonpath()` doesn't feel better. I'm ambivalent about either name. I'd vote for changing it then. Both because I think it might make it clearer that it's the common prefix of "foo/bar" and "food" is not "foo" and because, as I said, we might want such a function later and it would be unfortunate if the name was taken. Btw, I just checked and we already have `dirname()` and `stripdir()` functions that would match `commondir()`. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: martinvonz, yuja, mercurial-devel
> > `commondir()` would be fine as well. `commonpath()` doesn't feel better. I'm ambivalent about either nam> > I'd vote for changing it then. Both because I think it might make it clearer that it's the common prefix of "foo/bar" and "food" is not "foo" and because, as I said, we might want such a function later and it would be unfortunate if the name was taken. Btw, I just checked and we already have `dirname()` and `stripdir()` functions that would match `commondir()`. +1 for `commondir()`. It matches the actual behavior, which is the last filename part is always stripped.
yuja added a comment. > > `commondir()` would be fine as well. `commonpath()` doesn't feel better. I'm ambivalent about either nam> > I'd vote for changing it then. Both because I think it might make it clearer that it's the common prefix of "foo/bar" and "food" is not "foo" and because, as I said, we might want such a function later and it would be unfortunate if the name was taken. Btw, I just checked and we already have `dirname()` and `stripdir()` functions that would match `commondir()`. +1 for `commondir()`. It matches the actual behavior, which is the last filename part is always stripped. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D3439 To: joerg.sonnenberger, #hg-reviewers Cc: martinvonz, yuja, mercurial-devel
Patch
diff --git a/tests/test-template-filters.t b/tests/test-template-filters.t new file mode 100644 --- /dev/null +++ b/tests/test-template-filters.t @@ -0,0 +1,4 @@ + $ hg debugtemplate '{"foo/bar\nfoo/baz\n"|splitlines|commonprefix}\n' + foo + $ hg debugtemplate '{"foo/bar\nbar/baz"|splitlines|commonprefix}\n' + diff --git a/mercurial/templatefilters.py b/mercurial/templatefilters.py --- a/mercurial/templatefilters.py +++ b/mercurial/templatefilters.py @@ -99,6 +99,26 @@ """ return os.path.basename(path) +@templatefilter('commonprefix') +def commonprefix(filelist): + """List of text. Treats each list item as file name, and returns + the longest common directory prefix shared by all list items. + Returns the empty string if no common prefix exists. + For example, ["foo/bar/baz", "foo/baz/bar"] becomes "foo" and + ["foo/bar", "baz"] becomes "". + """ + def common(a, b): + while a != b: + if len(a) > len(b): + a = os.path.dirname(a) + else: + b = os.path.dirname(b) + return a + if not filelist: + return "" + dirlist = [os.path.dirname(os.path.normpath(f)) for f in filelist] + return reduce(common, dirlist) + @templatefilter('count') def count(i): """List or text. Returns the length as an integer."""