Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify how stars work in file patterns #1675

Merged
merged 2 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,15 @@ development at the same time, such as 4.5.x and 5.0.
Unreleased
----------

Nothing yet.
- The semantics of stars in file patterns has been clarified in the docs. A
leading or trailing star matches any number of path components, like a double
star would. This is different than the behavior of a star in the middle of a
pattern. This discrepancy was `identified by Sviatoslav Sydorenko
<starbad_>`_, who `provided patient detailed diagnosis <pull 1650_>`_ and
graciously agreed to a pragmatic resolution.

.. _starbad: https://github.com/nedbat/coveragepy/issues/1407#issuecomment-1631085209
.. _pull 1650: https://github.com/nedbat/coveragepy/pull/1650

.. scriv-start-here
Expand All @@ -32,7 +39,7 @@ Version 7.3.0 — 2023-08-12

- Added a :meth:`.Coverage.collect` context manager to start and stop coverage
data collection.

- Dropped support for Python 3.7.

- Fix: in unusual circumstances, SQLite cannot be set to asynchronous mode.
Expand Down
10 changes: 10 additions & 0 deletions doc/_static/coverage.css
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,16 @@ img.tideliftlogo {
background: #efc;
}

/* I'm not sure why I had to make this so specific to get it to take effect... */
div.rst-content div.document div.wy-table-responsive table.docutils.align-default tbody tr td {
vertical-align: top !important;
}

/* And this doesn't work, and I guess I just have to live with it. */
div.rst-content div.document div.wy-table-responsive table.docutils.align-default tbody tr td .line-block {
margin-bottom: 0 !important;
}

/* sphinx-code-tabs */

/* Some selectors here are extra-specific (.container) because this file comes
Expand Down
64 changes: 58 additions & 6 deletions doc/source.rst
Original file line number Diff line number Diff line change
Expand Up @@ -178,15 +178,67 @@ individual source lines. See :ref:`excluding` for details.
File patterns
-------------

File path patterns are used for include and omit, and for combining path
remapping. They follow common shell syntax:

- ``*`` matches any number of file name characters, not including the directory
separator.
File path patterns are used for :ref:`include <config_run_include>` and
:ref:`omit <config_run_omit>`, and for :ref:`combining path remapping
<cmd_combine_remapping>`. They follow common shell syntax:

- ``?`` matches a single file name character.

- ``**`` matches any number of nested directory names, including none.
- ``*`` matches any number of file name characters, not including the directory
separator. As a special case, if a pattern starts with ``*/``, it is treated
as ``**/``, and if a pattern ends with ``/*``, it is treated as ``/**``.

- ``**`` matches any number of nested directory names, including none. It must
be used as a full component of the path, not as part of a word: ``/**/`` is
allowed, but ``/a**/`` is not.

- Both ``/`` and ``\`` will match either a slash or a backslash, to make
cross-platform matching easier.

- A pattern with no directory separators matches the file name in any
directory.

Some examples:

.. list-table::
:widths: 20 20 20
:header-rows: 1

* - Pattern
- Matches
- Doesn't Match
* - ``a*.py``
- | anything.py
| sub1/sub2/another.py
- | cat.py
* - ``sub/*/*.py``
- | sub/a/main.py
| sub/b/another.py
- | sub/foo.py
| sub/m1/m2/foo.py
* - ``sub/**/*.py``
- | sub/something.py
| sub/a/main.py
| sub/b/another.py
| sub/m1/m2/foo.py
- | sub1/anything.py
| sub1/more/code/main.py
* - ``*/sub/*``
- | some/where/sub/more/something.py
| sub/hello.py
- | sub1/anything.py
* - ``*/sub*/*``
- | some/where/sub/more/something.py
| sub/hello.py
| sub1/anything.py
- | some/more/something.py
* - ``*/*sub/test_*.py``
- | some/where/sub/test_everything.py
| moresub/test_things.py
- | some/where/sub/more/test_everything.py
| more/test_things.py
* - ``*/*sub/*sub/**``
- | sub/sub/something.py
| asub/bsub/more/thing.py
| code/sub/sub/code.py
- | sub/something.py
18 changes: 17 additions & 1 deletion tests/test_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,24 @@ def globs_to_regex_params(
),
globs_to_regex_params(
["*/foo"], case_insensitive=False, partial=True,
matches=["abc/foo/hi.py", "foo/hi.py"],
matches=["abc/foo/hi.py", "foo/hi.py", "abc/def/foo/hi.py"],
nomatches=["abc/xfoo/hi.py"],
),
globs_to_regex_params(
["*c/foo"], case_insensitive=False, partial=True,
matches=["abc/foo/hi.py"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this will also work:

Suggested change
matches=["abc/foo/hi.py"],
matches=["abc/foo/hi.py", "stuff/abc/foo/hi.py"],

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wait, I see that a similar case is already in nomatches.. Let me reread the new doc explanation.

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't match. The next line includes nomatches=["def/abc/foo/hi.py"]

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I saw that after writing the comment.

nomatches=["abc/xfoo/hi.py", "foo/hi.py", "def/abc/foo/hi.py"],
),
globs_to_regex_params(
["foo/x*"], case_insensitive=False, partial=True,
matches=["foo/x", "foo/xhi.py", "foo/x/hi.py"],
nomatches=[],
),
globs_to_regex_params(
["foo/x*"], case_insensitive=False, partial=False,
matches=["foo/x", "foo/xhi.py"],
nomatches=["foo/x/hi.py"],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though, it's there in the source code, I forgot that this wouldn't match since some time has passed since I first opened my PR. So when I read the new explanation, I assumed this would match too. I think that adding more specific examples to the docs would really help to disambiguate them.

),
globs_to_regex_params(
["**/foo"],
matches=["foo", "hello/foo", "hi/there/foo"],
Expand Down Expand Up @@ -348,6 +363,7 @@ def test_glob_matcher(self) -> None:
(self.make_file("sub/file1.py"), True),
(self.make_file("sub/file2.c"), False),
(self.make_file("sub2/file3.h"), True),
(self.make_file("sub2/sub/file3.h"), True),
(self.make_file("sub3/file4.py"), True),
(self.make_file("sub3/file5.c"), False),
]
Expand Down