{"thread":{"id":"36212","subject":"[PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()","startedAt":"2014-03-19T00:35:35Z","lastAt":"2014-03-19T04:08:33Z","messageCount":5,"participants":["Brian Bourn","Eric Sunshine","babourn"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"237028","messageId":"CAM+=D-Ac2yL02-6qGQFuV_qz4FKt_6EGwRHk4+MLDnSb5CgucQ@mail.gmail.com","threadId":"36212","inReplyTo":null,"subject":"[PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()","fromName":"Brian Bourn","fromEmail":"ba.bourn@gmail.com","sentAt":"2014-03-19T00:35:35Z","receivedAt":"2014-03-19T00:35:35Z","isPatch":true,"sender":{"key":"ba.bourn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3165958?v=4"},"body":"this was done in order to implement the GSoC\nMicro project for diff-no-index.c this is the\nfirst patch importing dir.h, for the use of\nis_dot_or_dotdot(), and renaming\nread_directory() to read_directory_contents()\nin order to deal with the conflicting function\nin dir.h\n\nSigned-off-by: Brian Bourn \n---\n\nI plan on applying to GSoC 2014\n\n diff-no-index.c | 7 ++++---\n 1 file changed, 4 insertions(+), 3 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 8e10bff..ba915af 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_directory_contents(const char *path, struct string_list *list)\n {\n DIR *dir;\n struct dirent *e;\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_directory_contents(name1, &p1))\n return -1;\n- if (name2 && read_directory(name2, &p2)) {\n+ if (name2 && read_directory_contents(name2, &p2)) {\n string_list_clear(&p1, 0);\n return -1;\n }\n-- \n1.9.0\n"},{"id":"237033","messageId":"CAPig+cTkZPCMo6smKie4ZJP-Xu4rAFw6yPY-PYqs2Pniyww7MQ@mail.gmail.com","threadId":"36212","inReplyTo":"CAM+=D-Ac2yL02-6qGQFuV_qz4FKt_6EGwRHk4+MLDnSb5CgucQ@mail.gmail.com","subject":"Re: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory()","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-19T01:19:42Z","receivedAt":"2014-03-19T01:19:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Thanks for the submission. Comments below to give you a taste of the\nGit review process...\n\nOn Tue, Mar 18, 2014 at 8:35 PM, Brian Bourn wrote:\n> Subject: diff: Imported dir.h and renamed read_directory()\n\nUse imperative voice: \"import dir.h and rename read_directory()\"\n\n> this was done in order to implement the GSoC\n> Micro project for diff-no-index.c\n\nThis commentary won't be relevant to anyone reading the commit message\nmonths or years from now. Place it below the \"---\" line following your\nsign-off.\n\n> this is the\n> first patch importing dir.h, for the use of\n> is_dot_or_dotdot(), and renaming\n> read_directory() to read_directory_contents()\n> in order to deal with the conflicting function\n> in dir.h\n\nSince this preparatory change and the one which will follow are so\nclosely related, they should be submitted together as a two-patch\nseries.\n\nThe commit message itself it somewhat rambling. It would be clearer if\nyou could explain the problem precisely, then the solution. Perhaps\nsomething like this:\n\n It would be desirable to replace manual checking of \".\" or \"..\"\n in diff-no-index.c with is_dot_or_dotdot(), which is defined in\n dir.h, however, dir.h declares a read_directory() which conflicts\n with a (different) static read_directory() defined in\n diff-no-index.c. As a preparatory step, rename the local\n read_directory() to avoid the collision.\n\n> Signed-off-by: Brian Bourn \n> ---\n>\n> I plan on applying to GSoC 2014\n>\n> diff-no-index.c | 7 ++++---\n> 1 file changed, 4 insertions(+), 3 deletions(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8e10bff..ba915af 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\ndir.h is not needed for this patch, so including it here only confuses\nmatters. Include it instead when it is actually required by the\nfollow-up patch which uses is_dot_or_dotdot().\n\n(Was it intentional to place the new #include between diff.h and\ndiffcore.h? Just being curious; it's not particularly important.)\n\nOther than this, the patch looks sane.\n\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_directory_contents(const char *path, struct string_list *list)\n> {\n> DIR *dir;\n> struct dirent *e;\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_directory_contents(name1, &p1))\n> return -1;\n> - if (name2 && read_directory(name2, &p2)) {\n> + if (name2 && read_directory_contents(name2, &p2)) {\n> string_list_clear(&p1, 0);\n> return -1;\n> }\n> --\n> 1.9.0\n"},{"id":"237034","messageId":"1395192636735-7605956.post@n2.nabble.com","threadId":"36212","inReplyTo":"CAM+=D-Ac2yL02-6qGQFuV_qz4FKt_6EGwRHk4+MLDnSb5CgucQ@mail.gmail.com","subject":"[PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code","fromName":"babourn","fromEmail":"ba.bourn@gmail.com","sentAt":"2014-03-19T01:30:36Z","receivedAt":"2014-03-19T01:30:36Z","isPatch":true,"sender":{"key":"ba.bourn@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3165958?v=4"},"body":"in accordance with the GSoC Microproject implemented\nthe call is_dot_or_dotdot() in the code in order to\nfurther universalize the call to the function and\nincrease code continuity.\n\nSigned-off-by: Brian Bourn \n---\n diff-no-index.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex ba915af..44cce25 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,\nstruct 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--\n1.9.0\n\n\n\n--\nView this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html\nSent from the git mailing list archive at Nabble.com.\n"},{"id":"237036","messageId":"CAPig+cSxu1WAEg7YmnNn-kHZMj4AacRaVPMthgAwFN0GmrrYCg@mail.gmail.com","threadId":"36212","inReplyTo":"1395192636735-7605956.post@n2.nabble.com","subject":"Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-19T03:45:42Z","receivedAt":"2014-03-19T03:45:42Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 18, 2014 at 9:30 PM, babourn wrote:\n> Subject: diff: used is_dot_or_dotdot() in code\n\nUse imperative voice: \"use\" rather than \"used\"\n\n> in accordance with the GSoC Microproject implemented\n\nThis commentary will not have much meaning to someone reading the\ncommit log months or years from now. Place it below the \"---\" line\nfollowing your sign-off.\n\n> the call is_dot_or_dotdot() in the code in order to\n> further universalize the call to the function and\n> increase code continuity.\n\nIt should be sufficient to explain the patch by just saying:\n\n Subject: replace manual \".\"/\"..\" check with is_dot_or_dotdot()\n\nThe rest of the explanatory text can be dropped since it doesn't add\nanything (meaningful) beyond what the subject says.\n\n> Signed-off-by: Brian Bourn \n> ---\n> diff-no-index.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index ba915af..44cce25 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,\n> 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\nThe patch is severely whitespace-damaged. (Did you post it through Nabble?)\n\n> string_list_insert(list, e->d_name);\n>\n> closedir(dir);\n> --\n> 1.9.0\n>\n>\n>\n> --\n> View this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html\n> Sent from the git mailing list archive at Nabble.com.\n> --\n"},{"id":"237038","messageId":"CAPig+cQJm7MoftaGd5=p-ooLVQQguzhwsYHNFgjjzywyq-UC=A@mail.gmail.com","threadId":"36212","inReplyTo":"CAM+=D-CO9A4hYb+9tVf_cAOMqkTm3taBLPp0eHtFkuMUCFw2ng@mail.gmail.com","subject":"Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-19T04:08:33Z","receivedAt":"2014-03-19T04:08:33Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Tue, Mar 18, 2014 at 11:58 PM, Brian Bourn wrote:\n> On Tue, Mar 18, 2014 at 11:45 PM, Eric Sunshine \n> wrote:\n>> On Tue, Mar 18, 2014 at 9:30 PM, babourn wrote:\n>> > Subject: diff: used is_dot_or_dotdot() in code\n>> > Signed-off-by: Brian Bourn \n>> > ---\n>> > diff-no-index.c | 2 +-\n>> > 1 file changed, 1 insertion(+), 1 deletion(-)\n>> >\n>> > diff --git a/diff-no-index.c b/diff-no-index.c\n>> > index ba915af..44cce25 100644\n>> > --- a/diff-no-index.c\n>> > +++ b/diff-no-index.c\n>> > @@ -26,7 +26,7 @@ static int read_directory_contents(const char *path,\n>> > 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>>\n>> The patch is severely whitespace-damaged. (Did you post it through\n>> Nabble?)\n>\n> I did post through Nabble, My email with the patch didn't seem to be\n> going through.\n> should I keep trying to resend it through email to undo the whitspace\n> damage?\n\nIt's probably not necessary to try resending this version of the patch\nsince you'll (hopefully) be sending a newer version which takes\nreviewer comments into consideration.\n\nWhat method are you using to send the patches? git send-email?\nSomething other? This particular mailing list rejects HTML-formatted\nmessages, so that could be the culprit if you pasted the patch into\nyour email client. It's a good idea to try sending patches to yourself\nvia \"git send-email\". If you can get that to work successfully, then\nthey should be accepted by the mailing list when sent via the same\nmechanism.\n"}]}