{"thread":{"id":"36183","subject":"[PATCH] Rewrite the diff-no-index.c","startedAt":"2014-03-16T12:44:18Z","lastAt":"2014-03-17T10:43:10Z","messageCount":5,"participants":["ubuntu2012@126.com","Eric Sunshine","沈承恩","Matthieu Moy"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"236828","messageId":"1394973858-16505-1-git-send-email-ubuntu2012@126.com","threadId":"36183","inReplyTo":null,"subject":"[PATCH] Rewrite the diff-no-index.c","fromName":"","fromEmail":"ubuntu2012@126.com","sentAt":"2014-03-16T12:44:18Z","receivedAt":"2014-03-16T12:44:18Z","isPatch":true,"sender":{"key":"ubuntu2012@126.com","avatar":null},"body":"From: 沈承恩 \n\nI am sorry for that I send this agian.Last patch I have some error.(Maybe this time will like the previous).It is apply for GSOC\n\nSigned-off-by: 沈承恩 \n---\n diff-no-index.c | 5 +++--\n dir.h | 3 ++-\n 2 files changed, 5 insertions(+), 3 deletions(-)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 8e10bff..1fb0c0f 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -3,13 +3,14 @@\n * Copyright (c) 2007 by Johannes Schindelin\n * Copyright (c) 2008 by Junio C Hamano\n */\n-\n+#define EXIT\n #include \"cache.h\"\n #include \"color.h\"\n #include \"commit.h\"\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@@ -25,7 +26,7 @@ static int read_directory(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);\ndiff --git a/dir.h b/dir.h\nindex 55e5345..c0e45c8 100644\n--- a/dir.h\n+++ b/dir.h\n@@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,\n extern int within_depth(const char *name, int namelen, int depth, int max_depth);\n \n extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);\n+#ifndef EXIT\n extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);\n-\n+#endif\n extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,\n \t\t\t\t int *dtype, struct exclude_list *el);\n struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n-- \n1.7.9.5\n"},{"id":"236864","messageId":"CAPig+cRjGonML5NLyv3eqsKuM45O+whVCOQKNdj-E=zW9mRy4g@mail.gmail.com","threadId":"36183","inReplyTo":"1394973858-16505-1-git-send-email-ubuntu2012@126.com","subject":"Re: [PATCH] Rewrite the diff-no-index.c","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2014-03-17T09:13:38Z","receivedAt":"2014-03-17T09:13:38Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"Thanks for the resubmission. Comments below...\n\nOn Sun, Mar 16, 2014 at 8:44 AM, wrote:\n> From: 沈承恩 \n> Subject: [PATCH] Rewrite the diff-no-index.c\n\nThis is your second version of the patch, so you should say [PATCH v2].\n\nMost patches rewrite something, so \"rewrite\" in the subject does not\nconvey much. Better would be to explain what the patch does. For\ninstance:\n\n Subject: diff-no-index: replace manual \".\" & \"..\" check with\nis_dot_or_dotdot()\n\n> I am sorry for that I send this agian.Last patch I have some error.(Maybe this time will like the previous).It is apply for GSOC\n\nThis commentary is relevant to the ongoing email conversation but does\nnot belong in the commit message, so you should place it below the\n\"---\" line after your sign-off.\n\n> Signed-off-by: 沈承恩 \n> ---\n\nThis is where you would place commentary. It is also good etiquette to\ntell reviewers what changed in this version of the patch and to\nprovide a link to the previous version, like this [1].\n\n[1]: http://thread.gmane.org/gmane.comp.version-control.git/244093\n\n> diff-no-index.c | 5 +++--\n> dir.h | 3 ++-\n> 2 files changed, 5 insertions(+), 3 deletions(-)\n>\n> diff --git a/diff-no-index.c b/diff-no-index.c\n> index 8e10bff..1fb0c0f 100644\n> --- a/diff-no-index.c\n> +++ b/diff-no-index.c\n> @@ -3,13 +3,14 @@\n> * Copyright (c) 2007 by Johannes Schindelin\n> * Copyright (c) 2008 by Junio C Hamano\n> */\n> -\n> +#define EXIT\n\nThis change is non-obvious and should be explained in the commit\nmessage, otherwise reviewers will not understand its purpose.\n\nIn fact, you are doing this because you want to omit the declaration\nof read_directory() from dir.h when it is included in this file to\navoid conflict with the (different) read_directory() implementation in\nthis file. This is an ugly way to solve the problem. Renaming\nread_directory() in this file would be a much cleaner solution (but\nshould be done as a separate preparatory patch).\n\n> #include \"cache.h\"\n> #include \"color.h\"\n> #include \"commit.h\"\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> @@ -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\nThis logic is backward. Keep in mind the return value of strcmp() and\nthen think carefully about the expression 'strcmp(...) &&\nstrcmp(...)'.\n\n> string_list_insert(list, e->d_name);\n>\n> closedir(dir);\n> diff --git a/dir.h b/dir.h\n> index 55e5345..c0e45c8 100644\n> --- a/dir.h\n> +++ b/dir.h\n> @@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,\n> extern int within_depth(const char *name, int namelen, int depth, int max_depth);\n>\n> extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);\n> +#ifndef EXIT\n> extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);\n> -\n> +#endif\n\nSee above.\n\n> extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,\n> int *dtype, struct exclude_list *el);\n> struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n> --\n> 1.7.9.5\n"},{"id":"236875","messageId":"d6981b9.14361.144cf91cb20.Coremail.ubuntu2012@126.com","threadId":"36183","inReplyTo":"CAPig+cRjGonML5NLyv3eqsKuM45O+whVCOQKNdj-E=zW9mRy4g@mail.gmail.com","subject":"Re:Re: [PATCH] Rewrite the diff-no-index.c","fromName":"沈承恩","fromEmail":"ubuntu2012@126.com","sentAt":"2014-03-17T10:24:02Z","receivedAt":"2014-03-17T10:24:02Z","isPatch":true,"sender":{"key":"ubuntu2012@126.com","avatar":null},"body":"thank you for your comments\nAt 2014-03-17 17:13:38,\"Eric Sunshine\" wrote:\n>Thanks for the resubmission. Comments below...\n>\n>On Sun, Mar 16, 2014 at 8:44 AM, wrote:\n>> From: 沈承恩 \n>> Subject: [PATCH] Rewrite the diff-no-index.c\n>\n>This is your second version of the patch, so you should say [PATCH v2].\n>\n>Most patches rewrite something, so \"rewrite\" in the subject does not\n>convey much. Better would be to explain what the patch does. For\n>instance:\n>\n> Subject: diff-no-index: replace manual \".\" & \"..\" check with\n>is_dot_or_dotdot()\n>\n>> I am sorry for that I send this agian.Last patch I have some error.(Maybe this time will like the previous).It is apply for GSOC\n>\n>This commentary is relevant to the ongoing email conversation but does\n>not belong in the commit message, so you should place it below the\n>\"---\" line after your sign-off.\n>\n>> Signed-off-by: 沈承恩 \n>> ---\n>\n>This is where you would place commentary. It is also good etiquette to\n>tell reviewers what changed in this version of the patch and to\n>provide a link to the previous version, like this [1].\n>\n>[1]: http://thread.gmane.org/gmane.comp.version-control.git/244093\n>\n>> diff-no-index.c | 5 +++--\n>> dir.h | 3 ++-\n>> 2 files changed, 5 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/diff-no-index.c b/diff-no-index.c\n>> index 8e10bff..1fb0c0f 100644\n>> --- a/diff-no-index.c\n>> +++ b/diff-no-index.c\n>> @@ -3,13 +3,14 @@\n>> * Copyright (c) 2007 by Johannes Schindelin\n>> * Copyright (c) 2008 by Junio C Hamano\n>> */\n>> -\n>> +#define EXIT\n>\n>This change is non-obvious and should be explained in the commit\n>message, otherwise reviewers will not understand its purpose.\n>\n>In fact, you are doing this because you want to omit the declaration\n>of read_directory() from dir.h when it is included in this file to\n>avoid conflict with the (different) read_directory() implementation in\n>this file. This is an ugly way to solve the problem. Renaming\n>read_directory() in this file would be a much cleaner solution (but\n>should be done as a separate preparatory patch).\n>\n>> #include \"cache.h\"\n>> #include \"color.h\"\n>> #include \"commit.h\"\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>> @@ -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>\n>This logic is backward. Keep in mind the return value of strcmp() and\n>then think carefully about the expression 'strcmp(...) &&\n>strcmp(...)'.\n>\n>> string_list_insert(list, e->d_name);\n>>\n>> closedir(dir);\n>> diff --git a/dir.h b/dir.h\n>> index 55e5345..c0e45c8 100644\n>> --- a/dir.h\n>> +++ b/dir.h\n>> @@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,\n>> extern int within_depth(const char *name, int namelen, int depth, int max_depth);\n>>\n>> extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);\n>> +#ifndef EXIT\n>> extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);\n>> -\n>> +#endif\n>\n>See above.\n>\n>> extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,\n>> int *dtype, struct exclude_list *el);\n>> struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n>> --\n>> 1.7.9.5\n"},{"id":"236872","messageId":"57a096bd.145a2.144cf9c12d4.Coremail.ubuntu2012@126.com","threadId":"36183","inReplyTo":"CAPig+cRjGonML5NLyv3eqsKuM45O+whVCOQKNdj-E=zW9mRy4g@mail.gmail.com","subject":"Re:Re: [PATCH] Rewrite the diff-no-index.c","fromName":"沈承恩","fromEmail":"ubuntu2012@126.com","sentAt":"2014-03-17T10:35:16Z","receivedAt":"2014-03-17T10:35:16Z","isPatch":true,"sender":{"key":"ubuntu2012@126.com","avatar":null},"body":"Thank you for your comments.If I rename some function when I work on a large project like git,whether it will cause other error that I can not solve .So I use the ugly way for this reason.\nAt 2014-03-17 17:13:38,\"Eric Sunshine\" wrote:\n>Thanks for the resubmission. Comments below...\n>\n>On Sun, Mar 16, 2014 at 8:44 AM, wrote:\n>> From: 沈承恩 \n>> Subject: [PATCH] Rewrite the diff-no-index.c\n>\n>This is your second version of the patch, so you should say [PATCH v2].\n>\n>Most patches rewrite something, so \"rewrite\" in the subject does not\n>convey much. Better would be to explain what the patch does. For\n>instance:\n>\n> Subject: diff-no-index: replace manual \".\" & \"..\" check with\n>is_dot_or_dotdot()\n>\n>> I am sorry for that I send this agian.Last patch I have some error.(Maybe this time will like the previous).It is apply for GSOC\n>\n>This commentary is relevant to the ongoing email conversation but does\n>not belong in the commit message, so you should place it below the\n>\"---\" line after your sign-off.\n>\n>> Signed-off-by: 沈承恩 \n>> ---\n>\n>This is where you would place commentary. It is also good etiquette to\n>tell reviewers what changed in this version of the patch and to\n>provide a link to the previous version, like this [1].\n>\n>[1]: http://thread.gmane.org/gmane.comp.version-control.git/244093\n>\n>> diff-no-index.c | 5 +++--\n>> dir.h | 3 ++-\n>> 2 files changed, 5 insertions(+), 3 deletions(-)\n>>\n>> diff --git a/diff-no-index.c b/diff-no-index.c\n>> index 8e10bff..1fb0c0f 100644\n>> --- a/diff-no-index.c\n>> +++ b/diff-no-index.c\n>> @@ -3,13 +3,14 @@\n>> * Copyright (c) 2007 by Johannes Schindelin\n>> * Copyright (c) 2008 by Junio C Hamano\n>> */\n>> -\n>> +#define EXIT\n>\n>This change is non-obvious and should be explained in the commit\n>message, otherwise reviewers will not understand its purpose.\n>\n>In fact, you are doing this because you want to omit the declaration\n>of read_directory() from dir.h when it is included in this file to\n>avoid conflict with the (different) read_directory() implementation in\n>this file. This is an ugly way to solve the problem. Renaming\n>read_directory() in this file would be a much cleaner solution (but\n>should be done as a separate preparatory patch).\n>\n>> #include \"cache.h\"\n>> #include \"color.h\"\n>> #include \"commit.h\"\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>> @@ -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>\n>This logic is backward. Keep in mind the return value of strcmp() and\n>then think carefully about the expression 'strcmp(...) &&\n>strcmp(...)'.\n>\n>> string_list_insert(list, e->d_name);\n>>\n>> closedir(dir);\n>> diff --git a/dir.h b/dir.h\n>> index 55e5345..c0e45c8 100644\n>> --- a/dir.h\n>> +++ b/dir.h\n>> @@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,\n>> extern int within_depth(const char *name, int namelen, int depth, int max_depth);\n>>\n>> extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);\n>> +#ifndef EXIT\n>> extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);\n>> -\n>> +#endif\n>\n>See above.\n>\n>> extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,\n>> int *dtype, struct exclude_list *el);\n>> struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);\n>> --\n>> 1.7.9.5\n"},{"id":"236873","messageId":"282721457.578099.1395052990145.JavaMail.zimbra@imag.fr","threadId":"36183","inReplyTo":"57a096bd.145a2.144cf9c12d4.Coremail.ubuntu2012@126.com","subject":"Re: [PATCH] Rewrite the diff-no-index.c","fromName":"Matthieu Moy","fromEmail":"matthieu.moy@grenoble-inp.fr","sentAt":"2014-03-17T10:43:10Z","receivedAt":"2014-03-17T10:43:10Z","isPatch":true,"sender":{"key":"matthieu.moy@grenoble-inp.fr","avatar":"https://gravatar.com/avatar/72c8a2705971a25dfaff23cece15130d405685845d911aedd5667ace277f3fc5?d=mp&s=160"},"body":"----- Original Message -----\n> Thank you for your comments.If I rename some function when I work on a large\n> project like git,whether it will cause other error that I can not solve .So\n> I use the ugly way for this reason.\n\n(please, don't top-post on this list).\n\nThe read_directory to be renamed is a static function, called twice. I don't see any reason not to rename it.\n\n-- \nMatthieu Moy\nhttp://www-verimag.imag.fr/~moy/\n"}]}