Skip to content
88 changes: 74 additions & 14 deletions dvc/command/check_ignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from dvc.command import completion
from dvc.command.base import CmdBase, append_doc_link
from dvc.exceptions import DvcException
from dvc.prompt import ask

logger = logging.getLogger(__name__)

Expand All @@ -14,27 +15,71 @@ def __init__(self, args):
self.ignore_filter = self.repo.tree.dvcignore

def _show_results(self, result):
if result.match or self.args.non_matching:
if self.args.details:
logger.info("{}\t{}".format(result.patterns[-1], result.file))
else:
logger.info(result.file)
if not result.match and not self.args.non_matching:
return

def run(self):
if self.args.non_matching and not self.args.details:
raise DvcException("--non-matching is only valid with --details")
if self.args.details:
patterns = result.patterns
if not self.args.all:
patterns = patterns[-1:]

if self.args.quiet and self.args.details:
raise DvcException("cannot both --details and --quiet")
for pattern in patterns:
logger.info("{}\t{}".format(pattern, result.file))
else:
logger.info(result.file)

def _check_one_file(self, target):
result = self.ignore_filter.check_ignore(target)
self._show_results(result)
if result.match:
return 0
return 1

def _interactive_mode(self):
ret = 1
while True:
target = ask("")
if target == "":
logger.info(
"Empty string is not a valid pathspec. Please use . "
"instead if you meant to match all paths."
Comment on lines +44 to +45
Copy link
Contributor

@jorgeorpinel jorgeorpinel Aug 7, 2020

Choose a reason for hiding this comment

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

What is a pathspec?

Can users actually send an empty string (e.g. '')? Or does this happen when you don't send any args?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jorgeorpinel

  1. pathspec means the target to be checked
  2. It is in an interactive mode where users can type string as the target, and this happened when users type send an empty string.

Copy link
Contributor

@jorgeorpinel jorgeorpinel Aug 8, 2020

Choose a reason for hiding this comment

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

It is in an interactive mode

So maybe don't say anything and just go to the next > ? Seems intuitive enough that way. I just tried it and noticed it crashes after this error msg (i.e. it stops the interactive mode).

UPDATE: On Windows at least, there's no > chars in interactive mode, see iterative/dvc.org#1675 (review)

Copy link
Contributor

Choose a reason for hiding this comment

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

p.s. when I send an empty string as target i.e. dvc check-ignore '' it just checks it (never matches even if * is a pattern in .dvcignore) and this error isn't presented. Seems slightly inconsistent. I vote to just remove it altogether πŸ™‚

Copy link
Contributor

Choose a reason for hiding this comment

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

Extracted to #4361

)
break
if not self._check_one_file(target):
ret = 0
return ret

def _normal_mode(self):
ret = 1
for target in self.args.targets:
result = self.ignore_filter.check_ignore(target)
self._show_results(result)
if result.match:
if not self._check_one_file(target):
ret = 0
return ret

def _check_args(self):
if not self.args.stdin and not self.args.targets:
raise DvcException("`targets` or `--stdin` needed")

if self.args.stdin and self.args.targets:
raise DvcException("cannot have both `targets` and `--stdin`")

if self.args.non_matching and not self.args.details:
raise DvcException(
"`--non-matching` is only valid with `--details`"
)

if self.args.all and not self.args.details:
raise DvcException("`--all` is only valid with `--details`")

if self.args.quiet and self.args.details:
raise DvcException("cannot use both `--details` and `--quiet`")
Comment on lines +74 to +75
Copy link
Contributor

@jorgeorpinel jorgeorpinel Aug 7, 2020

Choose a reason for hiding this comment

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

Maybe --quiet should just win here?

Copy link
Contributor Author

@karajan1001 karajan1001 Aug 8, 2020

Choose a reason for hiding this comment

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

Maybe --quiet should just win here?

Yes, it used to be --verbose and --quiet. They might cause some problems, and had been fixed in #4304.
And for --details and --quiet we can just let --quiet win.


def run(self):
self._check_args()
if self.args.stdin:
return self._interactive_mode()
return self._normal_mode()


def add_parser(subparsers, parent_parser):
ADD_HELP = "Debug DVC ignore/exclude files"
Expand All @@ -61,9 +106,24 @@ def add_parser(subparsers, parent_parser):
help="Show the target paths which don’t match any pattern. "
"Only usable when `--details` is also employed",
)
parser.add_argument(
"--stdin",
action="store_true",
default=False,
help="Read pathnames from the standard input, one per line, "
"instead of from the command-line.",
)
parser.add_argument(
"-a",
"--all",
action="store_true",
default=False,
help="Show all of the patterns match the target paths. "
"Only usable when `--details` is also employed",
)
parser.add_argument(
"targets",
nargs="+",
nargs="*",
help="Exact or wildcard paths of files or directories to check "
"ignore patterns.",
).complete = completion.FILE
Expand Down
4 changes: 2 additions & 2 deletions dvc/prompt.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
logger = logging.getLogger(__name__)


def _ask(prompt, limited_to=None):
def ask(prompt, limited_to=None):
if not sys.stdout.isatty():
return None

Expand Down Expand Up @@ -39,7 +39,7 @@ def confirm(statement):
bool: whether or not specified statement was confirmed.
"""
prompt = f"{statement} [y/n]"
answer = _ask(prompt, limited_to=["yes", "no", "y", "n"])
answer = ask(prompt, limited_to=["yes", "no", "y", "n"])
return answer and answer.startswith("y")


Expand Down
62 changes: 44 additions & 18 deletions tests/func/test_check_ignore.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@


@pytest.mark.parametrize(
"file,ret,output", [("ignored", 0, "ignored\n"), ("not_ignored", 1, "")]
"file,ret,output", [("ignored", 0, True), ("not_ignored", 1, False)]
)
def test_check_ignore(tmp_dir, dvc, file, ret, output, caplog):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored")

assert main(["check-ignore", file]) == ret
assert output in caplog.text
assert (file in caplog.text) is output


@pytest.mark.parametrize(
Expand All @@ -39,26 +39,29 @@ def test_check_ignore_details(tmp_dir, dvc, file, ret, output, caplog):
assert output in caplog.text


@pytest.mark.parametrize(
"non_matching,output", [(["-n"], "::\tfile\n"), ([], "")]
)
def test_check_ignore_non_matching(tmp_dir, dvc, non_matching, output, caplog):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "other")

assert main(["check-ignore", "-d"] + non_matching + ["file"]) == 1
assert output in caplog.text


def test_check_ignore_non_matching_without_details(tmp_dir, dvc):
@pytest.mark.parametrize("non_matching", [True, False])
def test_check_ignore_non_matching(tmp_dir, dvc, non_matching, caplog):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "other")
if non_matching:
assert main(["check-ignore", "-d", "-n", "file"]) == 1
else:
assert main(["check-ignore", "-d", "file"]) == 1

assert main(["check-ignore", "-n", "file"]) == 255

assert ("::\tfile\n" in caplog.text) is non_matching

def test_check_ignore_details_with_quiet(tmp_dir, dvc):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "other")

assert main(["check-ignore", "-d", "-q", "file"]) == 255
@pytest.mark.parametrize(
"args",
[
["-n", "file"],
["-a", "file"],
["-q", "-d", "file"],
["--stdin", "file"],
[],
],
)
def test_check_ignore_error_args_cases(tmp_dir, dvc, args):
assert main(["check-ignore"] + args) == 255


@pytest.mark.parametrize("path,ret", [({"dir": {}}, 0), ({"dir": "files"}, 1)])
Expand Down Expand Up @@ -107,3 +110,26 @@ def test_check_sub_dir_ignore_file(tmp_dir, dvc, caplog):
with sub_dir.chdir():
assert main(["check-ignore", "-d", "foo"]) == 0
assert ".dvcignore:2:foo\tfoo" in caplog.text


def test_check_ignore_details_all(tmp_dir, dvc, caplog):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "f*\n!foo")

assert main(["check-ignore", "-d", "-a", "foo"]) == 0
assert "{}:1:f*\tfoo\n".format(DvcIgnore.DVCIGNORE_FILE) in caplog.text
assert "{}:2:!foo\tfoo\n".format(DvcIgnore.DVCIGNORE_FILE) in caplog.text


@pytest.mark.parametrize(
"file,ret,output", [("ignored", 0, True), ("not_ignored", 1, False)]
)
def test_check_ignore_stdin_mode(
tmp_dir, dvc, file, ret, output, caplog, mocker
):
tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "ignored")
mocker.patch("builtins.input", side_effect=[file, ""])
stdout_mock = mocker.patch("sys.stdout")
stdout_mock.isatty.return_value = True

assert main(["check-ignore", "--stdin"]) == ret
assert (file in caplog.text) is output