{"thread":{"id":"52141","subject":"[Outreachy] [PATCH] clone: rename static function `dir_exists()`.","startedAt":"2019-10-28T16:55:30Z","lastAt":"2019-10-29T20:14:32Z","messageCount":3,"participants":["Miriam Rubio","Junio C Hamano","Miriam R."],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"385008","messageId":"20191028165523.84333-1-mirucam@gmail.com","threadId":"52141","inReplyTo":null,"subject":"[Outreachy] [PATCH] clone: rename static function `dir_exists()`.","fromName":"Miriam Rubio","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-28T16:55:23Z","receivedAt":"2019-10-28T16:55:30Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"builtin/clone.c has a static function dir_exists() that\nchecks if a given path exists on the filesystem. It returns\ntrue (and it is correct for it to return true) when the\ngiven path exists as a non-directory (e.g. a regular file).\n\nThis is confusing. What the caller wants to check, and what\nthis function wants to return, is if the path exists, so\nrename it to path_exists().\n\nSigned-off-by: Miriam Rubio \n---\n builtin/clone.c | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex c46ee29f0a..b24f04cf33 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -899,7 +899,7 @@ static void dissociate_from_references(void)\n \tfree(alternates);\n }\n \n-static int dir_exists(const char *path)\n+static int path_exists(const char *path)\n {\n \tstruct stat sb;\n \treturn !stat(path, &sb);\n@@ -981,7 +981,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\tdir = guess_dir_name(repo_name, is_bundle, option_bare);\n \tstrip_trailing_slashes(dir);\n \n-\tdest_exists = dir_exists(dir);\n+\tdest_exists = path_exists(dir);\n \tif (dest_exists && !is_empty_dir(dir))\n \t\tdie(_(\"destination path '%s' already exists and is not \"\n \t\t\t\"an empty directory.\"), dir);\n@@ -992,7 +992,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t\twork_tree = NULL;\n \telse {\n \t\twork_tree = getenv(\"GIT_WORK_TREE\");\n-\t\tif (work_tree && dir_exists(work_tree))\n+\t\tif (work_tree && path_exists(work_tree))\n \t\t\tdie(_(\"working tree '%s' already exists.\"), work_tree);\n \t}\n \n@@ -1020,7 +1020,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (real_git_dir) {\n-\t\tif (dir_exists(real_git_dir))\n+\t\tif (path_exists(real_git_dir))\n \t\t\tjunk_git_dir_flags |= REMOVE_DIR_KEEP_TOPLEVEL;\n \t\tjunk_git_dir = real_git_dir;\n \t} else {\n-- \n2.21.0 (Apple Git-122)\n\n"},{"id":"385041","messageId":"xmqqimo86yon.fsf@gitster-ct.c.googlers.com","threadId":"52141","inReplyTo":"20191028165523.84333-1-mirucam@gmail.com","subject":"Re: [Outreachy] [PATCH] clone: rename static function `dir_exists()`.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2019-10-29T03:03:04Z","receivedAt":"2019-10-29T03:03:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Miriam Rubio writes:\n\n> builtin/clone.c has a static function dir_exists() that\n> checks if a given path exists on the filesystem. It returns\n> true (and it is correct for it to return true) when the\n> given path exists as a non-directory (e.g. a regular file).\n>\n> This is confusing. What the caller wants to check, and what\n> this function wants to return, is if the path exists, so\n> rename it to path_exists().\n>\n> Signed-off-by: Miriam Rubio \n> ---\n> builtin/clone.c | 8 ++++----\n> 1 file changed, 4 insertions(+), 4 deletions(-)\n\nWith a narrowed scope, the patch and its explanation are both\nperfect ;-)\n\nNow, with this localized change behind us, we may want to consider\nwhat to do with file_exists(path) that does not ensure the path is a\nfile. It would be a separate topic, and it is OK for the result\nafter such consideration to be \"let's not go further for now\". It\nalso is OK for it to be \"I am interested in digging further\", too.\n\nThanks. Will queue.\n"},{"id":"385104","messageId":"CAN7CjDAWvKXOp+z=d8g-7QRBxwSExNCmqnU87fXN_vsDhteOZg@mail.gmail.com","threadId":"52141","inReplyTo":"xmqqimo86yon.fsf@gitster-ct.c.googlers.com","subject":"Re: [Outreachy] [PATCH] clone: rename static function `dir_exists()`.","fromName":"Miriam R.","fromEmail":"mirucam@gmail.com","sentAt":"2019-10-29T20:14:20Z","receivedAt":"2019-10-29T20:14:32Z","isPatch":true,"sender":{"key":"mirucam@gmail.com","avatar":"https://avatars.githubusercontent.com/u/56339109?v=4"},"body":"Great! Thank you, Junio!\n\nEl mar., 29 oct. 2019 a las 4:03, Junio C Hamano () escribió:\n>\n> Miriam Rubio writes:\n>\n> > builtin/clone.c has a static function dir_exists() that\n> > checks if a given path exists on the filesystem. It returns\n> > true (and it is correct for it to return true) when the\n> > given path exists as a non-directory (e.g. a regular file).\n> >\n> > This is confusing. What the caller wants to check, and what\n> > this function wants to return, is if the path exists, so\n> > rename it to path_exists().\n> >\n> > Signed-off-by: Miriam Rubio \n> > ---\n> > builtin/clone.c | 8 ++++----\n> > 1 file changed, 4 insertions(+), 4 deletions(-)\n>\n> With a narrowed scope, the patch and its explanation are both\n> perfect ;-)\n>\n> Now, with this localized change behind us, we may want to consider\n> what to do with file_exists(path) that does not ensure the path is a\n> file. It would be a separate topic, and it is OK for the result\n> after such consideration to be \"let's not go further for now\". It\n> also is OK for it to be \"I am interested in digging further\", too.\n>\n> Thanks. Will queue.\n"}]}