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

Two new modes --all and --stdin for check-ignore #4323

Merged
merged 13 commits into from
Aug 6, 2020
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"
efiop marked this conversation as resolved.
Show resolved Hide resolved
)
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")
efiop marked this conversation as resolved.
Show resolved Hide resolved

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:
ret = self._interactive_mode()
else:
ret = self._normal_mode()
return ret
efiop marked this conversation as resolved.
Show resolved Hide resolved


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.",
jorgeorpinel marked this conversation as resolved.
Show resolved Hide resolved
)
karajan1001 marked this conversation as resolved.
Show resolved Hide resolved
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",
jorgeorpinel marked this conversation as resolved.
Show resolved Hide resolved
)
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