Fix globbing when there is a single match
See https://github.com/anishathalye/dotbot/issues/282 and https://github.com/anishathalye/dotbot/issues/315. This patch simplifies the implementation, removing special-case handling for the cases of zero matches and one match. Instead, any situation where `glob: true` is specified and the path contains a glob character (any of "?", "*", or "[") is treated as a glob case. The reason we check both `use_glob` and `_has_glob_chars()` is to more gracefully handle the case where the user has enabled globs by default, but most links do not contain glob characters and should not be treated as globs.
This commit is contained in:
parent
9f8fd76f32
commit
416f32f5fe
2 changed files with 49 additions and 70 deletions
|
@ -59,63 +59,39 @@ class Link(Plugin):
|
|||
self._log.lowinfo("Skipping %s" % destination)
|
||||
continue
|
||||
path = os.path.normpath(os.path.expandvars(os.path.expanduser(path)))
|
||||
if use_glob:
|
||||
if use_glob and self._has_glob_chars(path):
|
||||
glob_results = self._create_glob_results(path, exclude_paths)
|
||||
if len(glob_results) == 0:
|
||||
self._log.lowinfo("Globbing couldn't find anything matching " + str(path))
|
||||
continue
|
||||
if len(glob_results) == 1 and destination[-1] == "/":
|
||||
self._log.error("Ambiguous action requested.")
|
||||
self._log.error(
|
||||
"No wildcard in glob, directory use undefined: "
|
||||
+ destination
|
||||
+ " -> "
|
||||
+ str(glob_results)
|
||||
self._log.lowinfo("Globs from '" + path + "': " + str(glob_results))
|
||||
for glob_full_item in glob_results:
|
||||
# Find common dirname between pattern and the item:
|
||||
glob_dirname = os.path.dirname(os.path.commonprefix([path, glob_full_item]))
|
||||
glob_item = (
|
||||
glob_full_item
|
||||
if len(glob_dirname) == 0
|
||||
else glob_full_item[len(glob_dirname) + 1 :]
|
||||
)
|
||||
self._log.warning("Did you want to link the directory or into it?")
|
||||
success = False
|
||||
continue
|
||||
elif len(glob_results) == 1 and destination[-1] != "/":
|
||||
# perform a normal link operation
|
||||
# Add prefix to basepath, if provided
|
||||
if base_prefix:
|
||||
glob_item = base_prefix + glob_item
|
||||
# where is it going
|
||||
glob_link_destination = os.path.join(destination, glob_item)
|
||||
if create:
|
||||
success &= self._create(destination)
|
||||
success &= self._create(glob_link_destination)
|
||||
if force or relink:
|
||||
success &= self._delete(path, destination, relative, canonical_path, force)
|
||||
success &= self._link(
|
||||
path, destination, relative, canonical_path, ignore_missing
|
||||
)
|
||||
else:
|
||||
self._log.lowinfo("Globs from '" + path + "': " + str(glob_results))
|
||||
for glob_full_item in glob_results:
|
||||
# Find common dirname between pattern and the item:
|
||||
glob_dirname = os.path.dirname(os.path.commonprefix([path, glob_full_item]))
|
||||
glob_item = (
|
||||
glob_full_item
|
||||
if len(glob_dirname) == 0
|
||||
else glob_full_item[len(glob_dirname) + 1 :]
|
||||
)
|
||||
# Add prefix to basepath, if provided
|
||||
if base_prefix:
|
||||
glob_item = base_prefix + glob_item
|
||||
# where is it going
|
||||
glob_link_destination = os.path.join(destination, glob_item)
|
||||
if create:
|
||||
success &= self._create(glob_link_destination)
|
||||
if force or relink:
|
||||
success &= self._delete(
|
||||
glob_full_item,
|
||||
glob_link_destination,
|
||||
relative,
|
||||
canonical_path,
|
||||
force,
|
||||
)
|
||||
success &= self._link(
|
||||
success &= self._delete(
|
||||
glob_full_item,
|
||||
glob_link_destination,
|
||||
relative,
|
||||
canonical_path,
|
||||
ignore_missing,
|
||||
force,
|
||||
)
|
||||
success &= self._link(
|
||||
glob_full_item,
|
||||
glob_link_destination,
|
||||
relative,
|
||||
canonical_path,
|
||||
ignore_missing,
|
||||
)
|
||||
else:
|
||||
if create:
|
||||
success &= self._create(destination)
|
||||
|
@ -154,6 +130,9 @@ class Link(Plugin):
|
|||
else:
|
||||
return source
|
||||
|
||||
def _has_glob_chars(self, path):
|
||||
return any(i in path for i in "?*[")
|
||||
|
||||
def _glob(self, path):
|
||||
"""
|
||||
Wrap `glob.glob` in a python agnostic way, catching errors in usage.
|
||||
|
|
|
@ -270,7 +270,7 @@ def test_link_glob_4(home, dotfiles, run_dotbot):
|
|||
|
||||
|
||||
@pytest.mark.parametrize("path", ("foo", "foo/"))
|
||||
def test_link_glob_ambiguous_failure(path, home, dotfiles, run_dotbot):
|
||||
def test_link_glob_ignore_no_glob_chars(path, home, dotfiles, run_dotbot):
|
||||
"""Verify ambiguous link globbing fails."""
|
||||
|
||||
dotfiles.makedirs("foo")
|
||||
|
@ -286,28 +286,8 @@ def test_link_glob_ambiguous_failure(path, home, dotfiles, run_dotbot):
|
|||
}
|
||||
]
|
||||
)
|
||||
with pytest.raises(SystemExit):
|
||||
run_dotbot()
|
||||
assert not os.path.exists(os.path.join(home, "foo"))
|
||||
|
||||
|
||||
def test_link_glob_ambiguous_success(home, dotfiles, run_dotbot):
|
||||
"""Verify the case where ambiguous link globbing succeeds."""
|
||||
|
||||
dotfiles.makedirs("foo")
|
||||
dotfiles.write_config(
|
||||
[
|
||||
{
|
||||
"link": {
|
||||
"~/foo": {
|
||||
"path": "foo",
|
||||
"glob": True,
|
||||
}
|
||||
}
|
||||
}
|
||||
]
|
||||
)
|
||||
run_dotbot()
|
||||
assert os.path.islink(os.path.join(home, "foo"))
|
||||
assert os.path.exists(os.path.join(home, "foo"))
|
||||
|
||||
|
||||
|
@ -600,6 +580,26 @@ def test_link_glob_no_match(home, dotfiles, run_dotbot):
|
|||
run_dotbot()
|
||||
|
||||
|
||||
def test_link_glob_single_match(home, dotfiles, run_dotbot):
|
||||
"""Verify linking works even when glob matches exactly one file."""
|
||||
# regression test for https://github.com/anishathalye/dotbot/issues/282
|
||||
|
||||
dotfiles.write("foo/a", "apple")
|
||||
dotfiles.write_config(
|
||||
[
|
||||
{"defaults": {"link": {"glob": True, "create": True}}},
|
||||
{"link": {"~/.config/foo": "foo/*"}},
|
||||
]
|
||||
)
|
||||
run_dotbot()
|
||||
|
||||
assert not os.path.islink(os.path.join(home, ".config"))
|
||||
assert not os.path.islink(os.path.join(home, ".config", "foo"))
|
||||
assert os.path.islink(os.path.join(home, ".config", "foo", "a"))
|
||||
with open(os.path.join(home, ".config", "foo", "a")) as file:
|
||||
assert file.read() == "apple"
|
||||
|
||||
|
||||
@pytest.mark.skipif(
|
||||
"sys.platform[:5] == 'win32'",
|
||||
reason="These if commands won't run on Windows",
|
||||
|
|
Loading…
Reference in a new issue