On 8/9/2022 7:41 AM, Victoria Dye wrote:
...truncated...
It took me some time to understand what all of these (nested) conditions are
doing; one suggestion I have (feel free to ignore it, since it's really just
a matter of stylistic preference) is reduce some duplicate code/simplify the
change a bit by moving the sparse directory check into the main "if-else"
block:
Yes, I acknowledge this part is cluttered slightly ;)
------------->8------------->8------------->8------------->8------------->8-------------
diff --git a/builtin/mv.c b/builtin/mv.c
index 4729bb1a1a..1c1b9559f6 100644
--- a/builtin/mv.c
+++ b/builtin/mv.c
@@ -203,10 +203,11 @@ int cmd_mv(int argc, const char **argv, const char *prefix)
if (dest_path[0][0] == '\0')
/* special case: "." was normalized to "" */
destination = internal_prefix_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
- else if (!lstat(dest_path[0], &st) &&
- S_ISDIR(st.st_mode)) {
- dest_path[0] = add_slash(dest_path[0]);
- destination = internal_prefix_pathspec(dest_path[0], argv, argc, DUP_BASENAME);
+ else if ((!lstat(dest_path[0], &st) && S_ISDIR(st.st_mode)) ||
+ (!path_in_sparse_checkout(dst_w_slash, &the_index) &&
+ empty_dir_has_sparse_contents(dst_w_slash))) {
+ /* directory dest_path[0] exists on-disk or in the index */
+ destination = internal_prefix_pathspec(dst_w_slash, argv, argc, DUP_BASENAME);
} else {
if (argc != 1)
die(_("destination '%s' is not a directory"), dest_path[0]);
-------------8<-------------8<-------------8<-------------8<-------------8<-------------
It doesn't make for the prettiest condition (so your current approach might
be better in terms of readability) but, to me, it creates a clearer
distinction between the "if" and "else if" blocks (which handle the case
where 'dest_path[0]' is a directory), and the "else" block (which handles
the case where 'dest_path[0]' is a file).
I also find this way clearer! Thanks for the suggestion!
