{"thread":{"id":"36170","subject":"Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()","startedAt":"2014-03-15T06:44:18Z","lastAt":"2014-03-16T11:07:20Z","messageCount":2,"participants":["Akshay Aurora","Thomas Rast"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236777","messageId":"CAPGnZZn_Wz=LywVevmuXWQX4nO67EKMPazB8jKv-jTZ178=HdQ@mail.gmail.com","threadId":"36170","inReplyTo":"1394800759-87648-1-git-send-email-akshayaurora@yahoo.com","subject":"Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()","fromName":"Akshay Aurora","fromEmail":"akshayaurora@yahoo.com","sentAt":"2014-03-15T06:44:18Z","receivedAt":"2014-03-15T06:44:18Z","isPatch":true,"sender":{"key":"akshayaurora@yahoo.com","avatar":"https://gravatar.com/avatar/07e443d5c5589cab6f36170d55ab46619245b0d58bcb3d8e017e3652f614e856?d=mp&s=160"},"body":"Forgot to mention, this is one of the microprojects for GSoC this\nyear. Would be great to have some feedback.\n\nOn Fri, Mar 14, 2014 at 6:09 PM, Akshay Aurora <akshayaurora@yahoo.com> wrote:\n> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory()\n>\n> Signed-off-by: Akshay Aurora <akshayaurora@yahoo.com>\n> ---\n>  diff-no-index.c | 9 +++++----\n>  1 file changed, 5 insertions(+), 4 deletions(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8e10bff..2a17c9f 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -10,13 +10,14 @@\n>  #include \"blob.h\"\n>  #include \"tag.h\"\n>  #include \"diff.h\"\n> +#include \"dir.h\"\n>  #include \"diffcore.h\"\n>  #include \"revision.h\"\n>  #include \"log-tree.h\"\n>  #include \"builtin.h\"\n>  #include \"string-list.h\"\n>\n> -static int read_directory(const char *path, struct string_list *list)\n> +static int read_dir(const char *path, struct string_list *list)\n>  {\n>         DIR *dir;\n>         struct dirent *e;\n> @@ -25,7 +26,7 @@ static int read_directory(const char *path, struct string_list *list)\n>                 return error(\"Could not open directory %s\", path);\n>\n>         while ((e = readdir(dir)))\n> -               if (strcmp(\".\", e->d_name) && strcmp(\"..\", e->d_name))\n> +               if (!is_dot_or_dotdot(e->d_name))\n>                         string_list_insert(list, e->d_name);\n>\n>         closedir(dir);\n> @@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o,\n>                 int i1, i2, ret = 0;\n>                 size_t len1 = 0, len2 = 0;\n>\n> -               if (name1 && read_directory(name1, &p1))\n> +               if (name1 && read_dir(name1, &p1))\n>                         return -1;\n> -               if (name2 && read_directory(name2, &p2)) {\n> +               if (name2 && read_dir(name2, &p2)) {\n>                         string_list_clear(&p1, 0);\n>                         return -1;\n>                 }\n> --\n> 1.8.5.3\n>\n\n\n\n-- \nWith Thanks & Warm Regards\nAkshay Aurora\niakshay.net\n"},{"id":"236824","messageId":"87eh22xuzb.fsf@thomasrast.ch","threadId":"36170","inReplyTo":"CAPGnZZn_Wz=LywVevmuXWQX4nO67EKMPazB8jKv-jTZ178=HdQ@mail.gmail.com","subject":"Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()","fromName":"Thomas Rast","fromEmail":"tr@thomasrast.ch","sentAt":"2014-03-16T11:07:20Z","receivedAt":"2014-03-16T11:07:20Z","isPatch":true,"sender":{"key":"tr@thomasrast.ch","avatar":"https://avatars.githubusercontent.com/u/153510?v=4"},"body":"Akshay Aurora <akshayaurora@yahoo.com> writes:\n\n> Forgot to mention, this is one of the microprojects for GSoC this\n> year. Would be great to have some feedback.\n>\n> On Fri, Mar 14, 2014 at 6:09 PM, Akshay Aurora <akshayaurora@yahoo.com> wrote:\n>> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory()\n>>\n>> Signed-off-by: Akshay Aurora <akshayaurora@yahoo.com>\n\nHmm, the original mail never made it through to me, and gmane doesn't\nseem to have it either.  What happened here?  The headers suggest you\nused git-send-email, which should avoid these problems.  Can you dig up\nthe command and configuration you used to send it (but be careful to not\npost your password!)?\n\nOn the patch itself:\n\n> Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir()\n\nThe subject line is very long.  Aim for 50 characters, but certainly no\nmore than 72.\n\nYou are also conflating two separate things into one patch.  Try to\navoid doing that.\n\nFurthermore I am unconvinced that renaming a function from\nread_directory() to read_dir() is a win.  What are you trying to improve\nby the rename?  Renames are good if they improve clarity and/or\nconsistency, but read_dir() just risks confusion with readdir() and I\ncannot see any gain in consistency that would compensate for it.\n\n>> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory()\n\nPlease stick to the style outlined in SubmittingPatches:\n\n  The body should provide a meaningful commit message, which:\n\n    . explains the problem the change tries to solve, iow, what is wrong\n      with the current code without the change.\n\n    . justifies the way the change solves the problem, iow, why the\n      result with the change is better.\n\n    . alternate solutions considered but discarded, if any.\n\n  Describe your changes in imperative mood, e.g. \"make xyzzy do frotz\"\n  instead of \"[This patch] makes xyzzy do frotz\" or \"[I] changed xyzzy\n  to do frotz\", as if you are giving orders to the codebase to change\n  its behaviour.  Try to make sure your explanation can be understood\n  without external resources. Instead of giving a URL to a mailing list\n  archive, summarize the relevant points of the discussion.\n\nAlso, please wrap your commit messages at 72 characters.\n\n>> ---\n>>  diff-no-index.c | 9 +++++----\n\nThe microproject idea said\n\n  Rewrite diff-no-index.c:read_directory() to use\n  is_dot_or_dotdot(). Try to find other sites that can use that\n  function.\n\nAre there any others?\n\n-- \nThomas Rast\ntr@thomasrast.ch\n"}]}