{"thread":{"id":"36201","subject":"[PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","startedAt":"2014-03-17T21:30:23Z","lastAt":"2014-03-19T18:48:51Z","messageCount":8,"participants":["Hiroyuki Sano","Junio C Hamano","Eric Sunshine"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"236916","messageId":"1395091825-8330-1-git-send-email-sh19910711@gmail.com","threadId":"36201","inReplyTo":null,"subject":"[PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","fromName":"Hiroyuki Sano","fromEmail":"sh19910711@gmail.com","sentAt":"2014-03-17T21:30:23Z","receivedAt":"2014-03-17T21:30:23Z","isPatch":true,"sender":{"key":"sh19910711@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1015222?v=4"},"body":"Including \"dir.h\" in \"diff-no-index.c\", it causes a compile error, because\nthe same name function read_directory() is declared globally in \"dir.h\".\n\nThis change is to avoid conflicts as above.\n\nSigned-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n---\n diff-no-index.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 8e10bff..1ed5c9d 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -16,7 +16,7 @@\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 get_directory_list(const char *path, struct string_list *list)\n {\n \tDIR *dir;\n \tstruct dirent *e;\n@@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,\n \t\tint i1, i2, ret = 0;\n \t\tsize_t len1 = 0, len2 = 0;\n \n-\t\tif (name1 && read_directory(name1, &p1))\n+\t\tif (name1 && get_directory_list(name1, &p1))\n \t\t\treturn -1;\n-\t\tif (name2 && read_directory(name2, &p2)) {\n+\t\tif (name2 && get_directory_list(name2, &p2)) {\n \t\t\tstring_list_clear(&p1, 0);\n \t\t\treturn -1;\n \t\t}\n-- \n1.9.0\n"},{"id":"236917","messageId":"1395091825-8330-2-git-send-email-sh19910711@gmail.com","threadId":"36201","inReplyTo":"1395091825-8330-1-git-send-email-sh19910711@gmail.com","subject":"[PATCH 2/3][GSOC] diff: use is_dot_or_dotdot() instead of strcmp()","fromName":"Hiroyuki Sano","fromEmail":"sh19910711@gmail.com","sentAt":"2014-03-17T21:30:24Z","receivedAt":"2014-03-17T21:30:24Z","isPatch":true,"sender":{"key":"sh19910711@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1015222?v=4"},"body":"The is_dot_or_dotdot() is used to check if the string is either \".\" or \"..\".\n\nSigned-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n---\n diff-no-index.c | 3 ++-\n 1 file changed, 2 insertions(+), 1 deletion(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 1ed5c9d..ccd9270 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -11,6 +11,7 @@\n #include \"tag.h\"\n #include \"diff.h\"\n #include \"diffcore.h\"\n+#include \"dir.h\"\n #include \"revision.h\"\n #include \"log-tree.h\"\n #include \"builtin.h\"\n@@ -25,7 +26,7 @@ static int get_directory_list(const char *path, struct string_list *list)\n \t\treturn error(\"Could not open directory %s\", path);\n \n \twhile ((e = readdir(dir)))\n-\t\tif (strcmp(\".\", e->d_name) && strcmp(\"..\", e->d_name))\n+\t\tif (!is_dot_or_dotdot(e->d_name))\n \t\t\tstring_list_insert(list, e->d_name);\n \n \tclosedir(dir);\n-- \n1.9.0\n"},{"id":"236918","messageId":"1395091825-8330-3-git-send-email-sh19910711@gmail.com","threadId":"36201","inReplyTo":"1395091825-8330-1-git-send-email-sh19910711@gmail.com","subject":"[PATCH 3/3][GSOC] fsck: use is_dot_or_dotdot() instead of strcmp()","fromName":"Hiroyuki Sano","fromEmail":"sh19910711@gmail.com","sentAt":"2014-03-17T21:30:25Z","receivedAt":"2014-03-17T21:30:25Z","isPatch":true,"sender":{"key":"sh19910711@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1015222?v=4"},"body":"The is_dot_or_dotdot() is used to check if the string is either \".\" or \"..\".\n\nSigned-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n---\n fsck.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/fsck.c b/fsck.c\nindex b3022ad..c9d7784 100644\n--- a/fsck.c\n+++ b/fsck.c\n@@ -6,6 +6,7 @@\n #include \"commit.h\"\n #include \"tag.h\"\n #include \"fsck.h\"\n+#include \"dir.h\"\n \n static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)\n {\n@@ -171,10 +172,12 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n \t\t\thas_full_path = 1;\n \t\tif (!*name)\n \t\t\thas_empty_name = 1;\n-\t\tif (!strcmp(name, \".\"))\n-\t\t\thas_dot = 1;\n-\t\tif (!strcmp(name, \"..\"))\n-\t\t\thas_dotdot = 1;\n+\t\tif (is_dot_or_dotdot(name)) {\n+\t\t\tif (!name[1])\n+\t\t\t\thas_dot = 1;\n+\t\t\telse\n+\t\t\t\thas_dotdot = 1;\n+\t\t}\n \t\tif (!strcmp(name, \".git\"))\n \t\t\thas_dotgit = 1;\n \t\thas_zero_pad |= *(char *)desc.buffer == '0';\n-- \n1.9.0\n"},{"id":"236932","messageId":"xmqqy508a1up.fsf@gitster.dls.corp.google.com","threadId":"36201","inReplyTo":"1395091825-8330-1-git-send-email-sh19910711@gmail.com","subject":"Re: [PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T22:37:18Z","receivedAt":"2014-03-17T22:37:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hiroyuki Sano <sh19910711@gmail.com> writes:\n\n> Including \"dir.h\" in \"diff-no-index.c\", it causes a compile error, because\n> the same name function read_directory() is declared globally in \"dir.h\".\n>\n> This change is to avoid conflicts as above.\n>\n> Signed-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n> ---\n>  diff-no-index.c | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8e10bff..1ed5c9d 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -16,7 +16,7 @@\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 get_directory_list(const char *path, struct string_list *list)\n\nRenaming is a good idea but the new name sounds like you are\ngrabbing the names of directories, ignoring all the files, no?\n\n>  {\n>  \tDIR *dir;\n>  \tstruct dirent *e;\n> @@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,\n>  \t\tint i1, i2, ret = 0;\n>  \t\tsize_t len1 = 0, len2 = 0;\n>  \n> -\t\tif (name1 && read_directory(name1, &p1))\n> +\t\tif (name1 && get_directory_list(name1, &p1))\n>  \t\t\treturn -1;\n> -\t\tif (name2 && read_directory(name2, &p2)) {\n> +\t\tif (name2 && get_directory_list(name2, &p2)) {\n>  \t\t\tstring_list_clear(&p1, 0);\n>  \t\t\treturn -1;\n>  \t\t}\n"},{"id":"236933","messageId":"xmqqtxawa1r5.fsf@gitster.dls.corp.google.com","threadId":"36201","inReplyTo":"1395091825-8330-3-git-send-email-sh19910711@gmail.com","subject":"Re: [PATCH 3/3][GSOC] fsck: use is_dot_or_dotdot() instead of strcmp()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-17T22:39:26Z","receivedAt":"2014-03-17T22:39:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Hiroyuki Sano <sh19910711@gmail.com> writes:\n\n> The is_dot_or_dotdot() is used to check if the string is either \".\" or \"..\".\n>\n> Signed-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n> ---\n>  fsck.c | 11 +++++++----\n>  1 file changed, 7 insertions(+), 4 deletions(-)\n>\n> diff --git a/fsck.c b/fsck.c\n> index b3022ad..c9d7784 100644\n> --- a/fsck.c\n> +++ b/fsck.c\n> @@ -6,6 +6,7 @@\n>  #include \"commit.h\"\n>  #include \"tag.h\"\n>  #include \"fsck.h\"\n> +#include \"dir.h\"\n>  \n>  static int fsck_walk_tree(struct tree *tree, fsck_walk_func walk, void *data)\n>  {\n> @@ -171,10 +172,12 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)\n>  \t\t\thas_full_path = 1;\n>  \t\tif (!*name)\n>  \t\t\thas_empty_name = 1;\n> -\t\tif (!strcmp(name, \".\"))\n> -\t\t\thas_dot = 1;\n> -\t\tif (!strcmp(name, \"..\"))\n> -\t\t\thas_dotdot = 1;\n> +\t\tif (is_dot_or_dotdot(name)) {\n> +\t\t\tif (!name[1])\n> +\t\t\t\thas_dot = 1;\n> +\t\t\telse\n> +\t\t\t\thas_dotdot = 1;\n> +\t\t}\n\nIn what way is this an improvement?\n\nThis looks like \"because I was told to\", not \"because the resulting\ncode is better\" to me.\n\nThe other patch on diff-no-index looked sensible, though.\n\n\n\n>  \t\tif (!strcmp(name, \".git\"))\n>  \t\t\thas_dotgit = 1;\n>  \t\thas_zero_pad |= *(char *)desc.buffer == '0';\n"},{"id":"237054","messageId":"CAPig+cR4564tsCO0U9HWNF9JZk64oFjx=nAWq+1QpGWSUn7Acg@mail.gmail.com","threadId":"36201","inReplyTo":"1395091825-8330-1-git-send-email-sh19910711@gmail.com","subject":"Re: [PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-19T10:04:20Z","receivedAt":"2014-03-19T10:04:20Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Mar 17, 2014 at 5:30 PM, Hiroyuki Sano <sh19910711@gmail.com> wrote:\n> Including \"dir.h\" in \"diff-no-index.c\", it causes a compile error, because\n> the same name function read_directory() is declared globally in \"dir.h\".\n>\n> This change is to avoid conflicts as above.\n\nThis explanation is slightly lacking. Since dir.h is not currently\nincluded by this file, a person reading this patch may wonder why you\nneed to avoid conflict where there is none. Adding a short sentence\nsaying that a subsequent patch will include dir.h in order to access\nis_dot_or_dotdot() can avoid such confusion.\n\n> Signed-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n> ---\n>  diff-no-index.c | 6 +++---\n>  1 file changed, 3 insertions(+), 3 deletions(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8e10bff..1ed5c9d 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -16,7 +16,7 @@\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 get_directory_list(const char *path, struct string_list *list)\n>  {\n>         DIR *dir;\n>         struct dirent *e;\n> @@ -107,9 +107,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 && get_directory_list(name1, &p1))\n>                         return -1;\n> -               if (name2 && read_directory(name2, &p2)) {\n> +               if (name2 && get_directory_list(name2, &p2)) {\n>                         string_list_clear(&p1, 0);\n>                         return -1;\n>                 }\n> --\n> 1.9.0\n"},{"id":"237088","messageId":"xmqq4n2u5cif.fsf@gitster.dls.corp.google.com","threadId":"36201","inReplyTo":"xmqqy508a1up.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T17:22:48Z","receivedAt":"2014-03-19T17:22:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Hiroyuki Sano <sh19910711@gmail.com> writes:\n>\n>> Including \"dir.h\" in \"diff-no-index.c\", it causes a compile error, because\n>> the same name function read_directory() is declared globally in \"dir.h\".\n>>\n>> This change is to avoid conflicts as above.\n>>\n>> Signed-off-by: Hiroyuki Sano <sh19910711@gmail.com>\n>> ---\n>>  diff-no-index.c | 6 +++---\n>>  1 file changed, 3 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/diff-no-index.c b/diff-no-index.c\n>> index 8e10bff..1ed5c9d 100644\n>> --- a/diff-no-index.c\n>> +++ b/diff-no-index.c\n>> @@ -16,7 +16,7 @@\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 get_directory_list(const char *path, struct string_list *list)\n>\n> Renaming is a good idea but the new name sounds like you are\n> grabbing the names of directories, ignoring all the files, no?\n\nI am tempted to suggest, because this is an internal implementation\ndetail only visible to this narrow corner of the system, calling\nthis just \n\n\tstatic int ls(const char *path, struct string_list *result)\n\n;-)\n"},{"id":"237100","messageId":"xmqq1txy3tyk.fsf@gitster.dls.corp.google.com","threadId":"36201","inReplyTo":"xmqq4n2u5cif.fsf@gitster.dls.corp.google.com","subject":"Re: [PATCH 1/3][GSOC] diff: rename read_directory() to get_directory_list()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-03-19T18:48:51Z","receivedAt":"2014-03-19T18:48:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n>>> -static int read_directory(const char *path, struct string_list *list)\n>>> +static int get_directory_list(const char *path, struct string_list *list)\n>>\n>> Renaming is a good idea but the new name sounds like you are\n>> grabbing the names of directories, ignoring all the files, no?\n>\n> I am tempted to suggest, because this is an internal implementation\n> detail only visible to this narrow corner of the system, calling\n> this just \n>\n> \tstatic int ls(const char *path, struct string_list *result)\n>\n> ;-)\n\nScratch that one.\n\nI'll take read_directory_contents() from Brian Bourn, which\nessentially is the same patch.\n\nThanks.\n\n-- >8 --\nFrom: Brian Bourn <ba.bourn@gmail.com>\nDate: Wed, 19 Mar 2014 11:58:21 -0400\nSubject: [PATCH] diff-no-index: rename read_directory()\n\nIn the next patch, we will replace a manual checking of \".\" or \"..\"\nwith a call to is_dot_or_dotdot() defined in dir.h.  The private\nfunction read_directory() defined in this file will conflict with\nthe global function declared there when we do so.\n\nAs a preparatory step, rename the private read_directory() to avoid\nthe name collision.\n\nSigned-off-by: Brian Bourn <ba.bourn@gmail.com>\nHelped-by: Eric Sunshine <sunshine@sunshineco.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n diff-no-index.c | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 33e5982..3e4f47e 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -16,7 +16,7 @@\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_directory_contents(const char *path, struct string_list *list)\n {\n \tDIR *dir;\n \tstruct dirent *e;\n@@ -107,9 +107,9 @@ static int queue_diff(struct diff_options *o,\n \t\tint i1, i2, ret = 0;\n \t\tsize_t len1 = 0, len2 = 0;\n \n-\t\tif (name1 && read_directory(name1, &p1))\n+\t\tif (name1 && read_directory_contents(name1, &p1))\n \t\t\treturn -1;\n-\t\tif (name2 && read_directory(name2, &p2)) {\n+\t\tif (name2 && read_directory_contents(name2, &p2)) {\n \t\t\tstring_list_clear(&p1, 0);\n \t\t\treturn -1;\n \t\t}\n-- \n1.9.1-423-g4596e3a\n"}]}