Re:Re: [PATCH] Rewrite the diff-no-index.c

[沈承恩 <ubuntu2012@126.com>]

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.
At 2014-03-17 17:13:38,"Eric Sunshine" <sunshine@sunshineco.com> wrote:
>Thanks for the resubmission. Comments below...
>
>On Sun, Mar 16, 2014 at 8:44 AM,  <ubuntu2012@126.com> wrote:
>> From: 沈承恩 <ubuntu2012@126.com>
>> Subject: [PATCH] Rewrite the diff-no-index.c
>
>This is your second version of the patch, so you should say [PATCH v2].
>
>Most patches rewrite something, so "rewrite" in the subject does not
>convey much. Better would be to explain what the patch does. For
>instance:
>
>    Subject: diff-no-index: replace manual "." & ".." check with
>is_dot_or_dotdot()
>
>> 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
>
>This commentary is relevant to the ongoing email conversation but does
>not belong in the commit message, so you should place it below the
>"---" line after your sign-off.
>
>> Signed-off-by: 沈承恩 <ubuntu2012@126.com>
>> ---
>
>This is where you would place commentary. It is also good etiquette to
>tell reviewers what changed in this version of the patch and to
>provide a link to the previous version, like this [1].
>
>[1]: http://thread.gmane.org/gmane.comp.version-control.git/244093
>
>>  diff-no-index.c |    5 +++--
>>  dir.h           |    3 ++-
>>  2 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/diff-no-index.c b/diff-no-index.c
>> index 8e10bff..1fb0c0f 100644
>> --- a/diff-no-index.c
>> +++ b/diff-no-index.c
>> @@ -3,13 +3,14 @@
>>   * Copyright (c) 2007 by Johannes Schindelin
>>   * Copyright (c) 2008 by Junio C Hamano
>>   */
>> -
>> +#define EXIT
>
>This change is non-obvious and should be explained in the commit
>message, otherwise reviewers will not understand its purpose.
>
>In fact, you are doing this because you want to omit the declaration
>of read_directory() from dir.h when it is included in this file to
>avoid conflict with the (different) read_directory() implementation in
>this file. This is an ugly way to solve the problem. Renaming
>read_directory() in this file would be a much cleaner solution (but
>should be done as a separate preparatory patch).
>
>>  #include "cache.h"
>>  #include "color.h"
>>  #include "commit.h"
>>  #include "blob.h"
>>  #include "tag.h"
>>  #include "diff.h"
>> +#include "dir.h"
>>  #include "diffcore.h"
>>  #include "revision.h"
>>  #include "log-tree.h"
>> @@ -25,7 +26,7 @@ static int read_directory(const char *path, struct string_list *list)
>>                 return error("Could not open directory %s", path);
>>
>>         while ((e = readdir(dir)))
>> -               if (strcmp(".", e->d_name) && strcmp("..", e->d_name))
>> +               if (is_dot_or_dotdot(e->d_name))
>
>This logic is backward. Keep in mind the return value of strcmp() and
>then think carefully about the expression 'strcmp(...) &&
>strcmp(...)'.
>
>>                         string_list_insert(list, e->d_name);
>>
>>         closedir(dir);
>> diff --git a/dir.h b/dir.h
>> index 55e5345..c0e45c8 100644
>> --- a/dir.h
>> +++ b/dir.h
>> @@ -138,8 +138,9 @@ extern int match_pathspec(const struct pathspec *pathspec,
>>  extern int within_depth(const char *name, int namelen, int depth, int max_depth);
>>
>>  extern int fill_directory(struct dir_struct *dir, const struct pathspec *pathspec);
>> +#ifndef EXIT
>>  extern int read_directory(struct dir_struct *, const char *path, int len, const struct pathspec *pathspec);
>> -
>> +#endif
>
>See above.
>
>>  extern int is_excluded_from_list(const char *pathname, int pathlen, const char *basename,
>>                                  int *dtype, struct exclude_list *el);
>>  struct dir_entry *dir_add_ignored(struct dir_struct *dir, const char *pathname, int len);
>> --
>> 1.7.9.5