From aa27920c44cedfdcaed31d2871fcf8b255cf6490 Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Mon, 3 Aug 2020 19:42:56 +0800 Subject: [PATCH 01/13] Two new mode `-all` and `--stdin` for check-ignore Fix #4321 Introduce new arguments --all for dvc check-ignore Add tests for dvc check-ignore --all --- dvc/command/check_ignore.py | 74 ++++++++++++++++++++++++++++----- dvc/prompt.py | 9 ++++ tests/func/test_check_ignore.py | 21 +++++----- 3 files changed, 83 insertions(+), 21 deletions(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index ef3f1250bb..8840dc57ba 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -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 path_input logger = logging.getLogger(__name__) @@ -14,25 +15,61 @@ 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 + + if self.args.details: + patterns = result.patterns + if not self.args.all: + patterns = patterns[-1:] + + 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 = path_input() + if target == "": + logger.info( + "empty string is not a valid pathspec. please use . " + "instead if you meant to match all paths" + ) + break + if not self._check_one_file(target): + ret = 0 + return ret + + def _normal_mode(self): + ret = 1 + for target in self.args.targets: + if not self._check_one_file(target): + ret = 0 + return ret 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.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 both --details and --quiet") - ret = 1 - for target in self.args.targets: - result = self.ignore_filter.check_ignore(target) - self._show_results(result) - if result.match: - ret = 0 + if self.args.stdin: + ret = self._interactive_mode() + else: + ret = self._normal_mode() return ret @@ -61,6 +98,21 @@ 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="+", diff --git a/dvc/prompt.py b/dvc/prompt.py index 285f6f62dc..8af245473d 100644 --- a/dvc/prompt.py +++ b/dvc/prompt.py @@ -54,3 +54,12 @@ def password(statement): """ logger.info(f"{statement}: ") return getpass("") + + +def path_input(): + """Ask the user for a path. + + Returns: + str: path entered by the user. + """ + return _ask("") diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index b53f72f531..214cab629a 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -49,16 +49,9 @@ def test_check_ignore_non_matching(tmp_dir, dvc, non_matching, output, caplog): assert output in caplog.text -def test_check_ignore_non_matching_without_details(tmp_dir, dvc): - tmp_dir.gen(DvcIgnore.DVCIGNORE_FILE, "other") - - assert main(["check-ignore", "-n", "file"]) == 255 - - -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"], ["-a"], ["-q", "-d"]]) +def test_check_ignore_error_args_cases(tmp_dir, dvc, args): + assert main(["check-ignore"] + args + ["file"]) == 255 @pytest.mark.parametrize("path,ret", [({"dir": {}}, 0), ({"dir": "files"}, 1)]) @@ -107,3 +100,11 @@ 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 From fc40747c75bfeea5143260a22a3a4e155e2df56b Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Tue, 4 Aug 2020 22:46:25 +0800 Subject: [PATCH 02/13] `--stdin` and targets only one 1. `--stdin` and targets can and only can have one 2. more tests --- dvc/command/check_ignore.py | 12 ++++++++++-- tests/func/test_check_ignore.py | 13 +++++++++++-- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 8840dc57ba..47aee6bd93 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -56,7 +56,13 @@ def _normal_mode(self): ret = 0 return ret - def run(self): + 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 targets and --stdin both") + if self.args.non_matching and not self.args.details: raise DvcException("--non-matching is only valid with --details") @@ -66,6 +72,8 @@ def run(self): if self.args.quiet and self.args.details: raise DvcException("cannot both --details and --quiet") + def run(self): + self._check_args() if self.args.stdin: ret = self._interactive_mode() else: @@ -115,7 +123,7 @@ def add_parser(subparsers, parent_parser): ) parser.add_argument( "targets", - nargs="+", + nargs="*", help="Exact or wildcard paths of files or directories to check " "ignore patterns.", ).complete = completion.FILE diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 214cab629a..6e03ceffc5 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -49,9 +49,18 @@ def test_check_ignore_non_matching(tmp_dir, dvc, non_matching, output, caplog): assert output in caplog.text -@pytest.mark.parametrize("args", [["-n"], ["-a"], ["-q", "-d"]]) +@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 + ["file"]) == 255 + assert main(["check-ignore"] + args) == 255 @pytest.mark.parametrize("path,ret", [({"dir": {}}, 0), ({"dir": "files"}, 1)]) From 489dd16f52aa28b098699c3a523d861277bc81ae Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Wed, 5 Aug 2020 09:46:11 +0800 Subject: [PATCH 03/13] New tests for stdin mode and solve a problem 1. Add a new tests with mock stdin for interactive mode 2. Solve the problem of `assert "" in caplog.text` in tests always `True`. --- tests/func/test_check_ignore.py | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/tests/func/test_check_ignore.py b/tests/func/test_check_ignore.py index 6e03ceffc5..eba86bc98f 100644 --- a/tests/func/test_check_ignore.py +++ b/tests/func/test_check_ignore.py @@ -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( @@ -39,14 +39,15 @@ 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): +@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", "-d"] + non_matching + ["file"]) == 1 - assert output in caplog.text + assert ("::\tfile\n" in caplog.text) is non_matching @pytest.mark.parametrize( @@ -117,3 +118,18 @@ def test_check_ignore_details_all(tmp_dir, dvc, caplog): 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 From 83d043e6ca5a675ec56b4c9ffae723c5e8b7af1d Mon Sep 17 00:00:00 2001 From: karajan1001 Date: Thu, 6 Aug 2020 21:01:16 +0800 Subject: [PATCH 04/13] Remove path_input interface 1. remove path_input and make _ask to a public method instead --- dvc/command/check_ignore.py | 4 ++-- dvc/prompt.py | 13 ++----------- 2 files changed, 4 insertions(+), 13 deletions(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 47aee6bd93..1347b2a58c 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -4,7 +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 path_input +from dvc.prompt import ask logger = logging.getLogger(__name__) @@ -38,7 +38,7 @@ def _check_one_file(self, target): def _interactive_mode(self): ret = 1 while True: - target = path_input() + target = ask("") if target == "": logger.info( "empty string is not a valid pathspec. please use . " diff --git a/dvc/prompt.py b/dvc/prompt.py index 8af245473d..cb12a73b93 100644 --- a/dvc/prompt.py +++ b/dvc/prompt.py @@ -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 @@ -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") @@ -54,12 +54,3 @@ def password(statement): """ logger.info(f"{statement}: ") return getpass("") - - -def path_input(): - """Ask the user for a path. - - Returns: - str: path entered by the user. - """ - return _ask("") From dbad2c604bcf36fa4efa671bc5ea496dc761e949 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:52:09 +0300 Subject: [PATCH 05/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 1347b2a58c..bde7ce0f46 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -41,7 +41,7 @@ def _interactive_mode(self): target = ask("") if target == "": logger.info( - "empty string is not a valid pathspec. please use . " + "Empty string is not a valid pathspec. Please use . " "instead if you meant to match all paths" ) break From e1eec42c3ef5ac7e6d3d00fd12fa81a7c8f89ada Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:53:10 +0300 Subject: [PATCH 06/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index bde7ce0f46..4dfbf2f410 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -61,7 +61,7 @@ def _check_args(self): raise DvcException("targets or --stdin needed") if self.args.stdin and self.args.targets: - raise DvcException("cannot have targets and --stdin both") + 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") From 5bc66a4a42b4e3545d79602eb7f33419b23c9917 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:53:43 +0300 Subject: [PATCH 07/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 4dfbf2f410..b28eedf6cb 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -58,7 +58,7 @@ def _normal_mode(self): def _check_args(self): if not self.args.stdin and not self.args.targets: - raise DvcException("targets or --stdin needed") + raise DvcException("`targets` or `--stdin` needed") if self.args.stdin and self.args.targets: raise DvcException("cannot have both `targets` and `--stdin`") From f10cfeda2f6b5b57260398810210f90c25691df1 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:54:02 +0300 Subject: [PATCH 08/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index b28eedf6cb..7869b518c2 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -67,7 +67,7 @@ def _check_args(self): 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") + raise DvcException("`--all` is only valid with `--details`") if self.args.quiet and self.args.details: raise DvcException("cannot both --details and --quiet") From b51f6d0a69e4adf582fa2fd58da5ddd45a64e9bd Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:54:41 +0300 Subject: [PATCH 09/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 7869b518c2..3d73127ca8 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -70,7 +70,7 @@ def _check_args(self): raise DvcException("`--all` is only valid with `--details`") if self.args.quiet and self.args.details: - raise DvcException("cannot both --details and --quiet") + raise DvcException("cannot use both `--details` and `--quiet`") def run(self): self._check_args() From edba25bd33ca4a37e7962ebd793e40658404804d Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:55:32 +0300 Subject: [PATCH 10/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 3d73127ca8..d5f61a3ae0 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -75,10 +75,8 @@ def _check_args(self): def run(self): self._check_args() if self.args.stdin: - ret = self._interactive_mode() - else: - ret = self._normal_mode() - return ret + return self._interactive_mode() + return self._normal_mode() def add_parser(subparsers, parent_parser): From 002d9425347080e3176fd23a70e8ec7556bf6d19 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:55:42 +0300 Subject: [PATCH 11/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index d5f61a3ae0..0a9ac63efb 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -42,7 +42,7 @@ def _interactive_mode(self): if target == "": logger.info( "Empty string is not a valid pathspec. Please use . " - "instead if you meant to match all paths" + "instead if you meant to match all paths." ) break if not self._check_one_file(target): From a5bb82db4a16ff21b1aa9fbb31bdaaad368b9dc1 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 20:56:26 +0300 Subject: [PATCH 12/13] Update dvc/command/check_ignore.py --- dvc/command/check_ignore.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 0a9ac63efb..06c45f5c12 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -64,7 +64,7 @@ def _check_args(self): 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") + 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`") From a504fa3ba80812385071e3dad0ec86ae1c108df7 Mon Sep 17 00:00:00 2001 From: Ruslan Kuprieiev Date: Thu, 6 Aug 2020 21:04:31 +0300 Subject: [PATCH 13/13] fix styling --- dvc/command/check_ignore.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/dvc/command/check_ignore.py b/dvc/command/check_ignore.py index 06c45f5c12..e28f4f19b4 100644 --- a/dvc/command/check_ignore.py +++ b/dvc/command/check_ignore.py @@ -64,7 +64,9 @@ def _check_args(self): 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`") + 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`")