Skip to content

Commit

Permalink
Fix globbing when there is a single match
Browse files Browse the repository at this point in the history
See anishathalye#282 and
anishathalye#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.
  • Loading branch information
anishathalye committed Jul 9, 2023
1 parent 9f8fd76 commit 416f32f
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 70 deletions.
75 changes: 27 additions & 48 deletions dotbot/plugins/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,63 +59,39 @@ def _process_links(self, links):
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)
Expand Down Expand Up @@ -154,6 +130,9 @@ def _default_source(self, destination, source):
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.
Expand Down
44 changes: 22 additions & 22 deletions tests/test_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand All @@ -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"))


Expand Down Expand Up @@ -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",
Expand Down

0 comments on commit 416f32f

Please sign in to comment.