# [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory() 5 messages from 2014-03-19 to 2014-03-19. Participants: Brian Bourn, Eric Sunshine, babourn. Thread: https://gitlist.dev/t/36212 ## Brian Bourn, 2014-03-19 00:35 Subject: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory() Message-ID: URL: https://gitlist.dev/e/CAM%2B%3DD-Ac2yL02-6qGQFuV_qz4FKt_6EGwRHk4%2BMLDnSb5CgucQ%40mail.gmail.com ``` this was done in order to implement the GSoC Micro project for diff-no-index.c this is the first patch importing dir.h, for the use of is_dot_or_dotdot(), and renaming read_directory() to read_directory_contents() in order to deal with the conflicting function in dir.h Signed-off-by: Brian Bourn --- I plan on applying to GSoC 2014 diff-no-index.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/diff-no-index.c b/diff-no-index.c index 8e10bff..ba915af 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -10,13 +10,14 @@ #include "blob.h" #include "tag.h" #include "diff.h" +#include "dir.h" #include "diffcore.h" #include "revision.h" #include "log-tree.h" #include "builtin.h" #include "string-list.h" -static int read_directory(const char *path, struct string_list *list) +static int read_directory_contents(const char *path, struct string_list *list) { DIR *dir; struct dirent *e; @@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o, int i1, i2, ret = 0; size_t len1 = 0, len2 = 0; - if (name1 && read_directory(name1, &p1)) + if (name1 && read_directory_contents(name1, &p1)) return -1; - if (name2 && read_directory(name2, &p2)) { + if (name2 && read_directory_contents(name2, &p2)) { string_list_clear(&p1, 0); return -1; } -- 1.9.0 ``` ## Eric Sunshine, 2014-03-19 01:19 Subject: Re: [PATCH] [GSoC 2014]diff: Imported dir.h and renamed read_directory() Message-ID: URL: https://gitlist.dev/e/CAPig%2BcTkZPCMo6smKie4ZJP-Xu4rAFw6yPY-PYqs2Pniyww7MQ%40mail.gmail.com In-Reply-To: ``` Thanks for the submission. Comments below to give you a taste of the Git review process... On Tue, Mar 18, 2014 at 8:35 PM, Brian Bourn wrote: > Subject: diff: Imported dir.h and renamed read_directory() Use imperative voice: "import dir.h and rename read_directory()" > this was done in order to implement the GSoC > Micro project for diff-no-index.c This commentary won't be relevant to anyone reading the commit message months or years from now. Place it below the "---" line following your sign-off. > this is the > first patch importing dir.h, for the use of > is_dot_or_dotdot(), and renaming > read_directory() to read_directory_contents() > in order to deal with the conflicting function > in dir.h Since this preparatory change and the one which will follow are so closely related, they should be submitted together as a two-patch series. The commit message itself it somewhat rambling. It would be clearer if you could explain the problem precisely, then the solution. Perhaps something like this: It would be desirable to replace manual checking of "." or ".." in diff-no-index.c with is_dot_or_dotdot(), which is defined in dir.h, however, dir.h declares a read_directory() which conflicts with a (different) static read_directory() defined in diff-no-index.c. As a preparatory step, rename the local read_directory() to avoid the collision. > Signed-off-by: Brian Bourn > --- > > I plan on applying to GSoC 2014 > > diff-no-index.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 8e10bff..ba915af 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -10,13 +10,14 @@ > #include "blob.h" > #include "tag.h" > #include "diff.h" > +#include "dir.h" > #include "diffcore.h" dir.h is not needed for this patch, so including it here only confuses matters. Include it instead when it is actually required by the follow-up patch which uses is_dot_or_dotdot(). (Was it intentional to place the new #include between diff.h and diffcore.h? Just being curious; it's not particularly important.) Other than this, the patch looks sane. > #include "revision.h" > #include "log-tree.h" > #include "builtin.h" > #include "string-list.h" > > -static int read_directory(const char *path, struct string_list *list) > +static int read_directory_contents(const char *path, struct string_list *list) > { > DIR *dir; > struct dirent *e; > @@ -107,9 +108,9 @@ static int queue_diff(struct diff_options *o, > int i1, i2, ret = 0; > size_t len1 = 0, len2 = 0; > > - if (name1 && read_directory(name1, &p1)) > + if (name1 && read_directory_contents(name1, &p1)) > return -1; > - if (name2 && read_directory(name2, &p2)) { > + if (name2 && read_directory_contents(name2, &p2)) { > string_list_clear(&p1, 0); > return -1; > } > -- > 1.9.0 ``` ## babourn, 2014-03-19 01:30 Subject: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code Message-ID: <1395192636735-7605956.post@n2.nabble.com> URL: https://gitlist.dev/e/1395192636735-7605956.post%40n2.nabble.com In-Reply-To: ``` in accordance with the GSoC Microproject implemented the call is_dot_or_dotdot() in the code in order to further universalize the call to the function and increase code continuity. Signed-off-by: Brian Bourn ---  diff-no-index.c | 2 +-  1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diff-no-index.c b/diff-no-index.c index ba915af..44cce25 100644 --- a/diff-no-index.c +++ b/diff-no-index.c @@ -26,7 +26,7 @@ static int read_directory_contents(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))   string_list_insert(list, e->d_name);   closedir(dir); -- 1.9.0 -- View this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html Sent from the git mailing list archive at Nabble.com. ``` ## Eric Sunshine, 2014-03-19 03:45 Subject: Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code Message-ID: URL: https://gitlist.dev/e/CAPig%2BcSxu1WAEg7YmnNn-kHZMj4AacRaVPMthgAwFN0GmrrYCg%40mail.gmail.com In-Reply-To: <1395192636735-7605956.post@n2.nabble.com> ``` On Tue, Mar 18, 2014 at 9:30 PM, babourn wrote: > Subject: diff: used is_dot_or_dotdot() in code Use imperative voice: "use" rather than "used" > in accordance with the GSoC Microproject implemented This commentary will not have much meaning to someone reading the commit log months or years from now. Place it below the "---" line following your sign-off. > the call is_dot_or_dotdot() in the code in order to > further universalize the call to the function and > increase code continuity. It should be sufficient to explain the patch by just saying: Subject: replace manual "."/".." check with is_dot_or_dotdot() The rest of the explanatory text can be dropped since it doesn't add anything (meaningful) beyond what the subject says. > Signed-off-by: Brian Bourn > --- > diff-no-index.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index ba915af..44cce25 100644 > --- a/diff-no-index.c > +++ b/diff-no-index.c > @@ -26,7 +26,7 @@ static int read_directory_contents(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)) The patch is severely whitespace-damaged. (Did you post it through Nabble?) > string_list_insert(list, e->d_name); > > closedir(dir); > -- > 1.9.0 > > > > -- > View this message in context: http://git.661346.n2.nabble.com/PATCH-GSoC-2014-diff-Imported-dir-h-and-renamed-read-directory-tp7605950p7605956.html > Sent from the git mailing list archive at Nabble.com. > -- ``` ## Eric Sunshine, 2014-03-19 04:08 Subject: Re: [PATCH 2/2][GSoC 2014] diff: used is_dot_or_dotdot() in code Message-ID: URL: https://gitlist.dev/e/CAPig%2BcQJm7MoftaGd5%3Dp-ooLVQQguzhwsYHNFgjjzywyq-UC%3DA%40mail.gmail.com In-Reply-To: ``` On Tue, Mar 18, 2014 at 11:58 PM, Brian Bourn wrote: > On Tue, Mar 18, 2014 at 11:45 PM, Eric Sunshine > wrote: >> On Tue, Mar 18, 2014 at 9:30 PM, babourn wrote: >> > Subject: diff: used is_dot_or_dotdot() in code >> > Signed-off-by: Brian Bourn >> > --- >> > diff-no-index.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/diff-no-index.c b/diff-no-index.c >> > index ba915af..44cce25 100644 >> > --- a/diff-no-index.c >> > +++ b/diff-no-index.c >> > @@ -26,7 +26,7 @@ static int read_directory_contents(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)) >> >> The patch is severely whitespace-damaged. (Did you post it through >> Nabble?) > > I did post through Nabble, My email with the patch didn't seem to be > going through. > should I keep trying to resend it through email to undo the whitspace > damage? It's probably not necessary to try resending this version of the patch since you'll (hopefully) be sending a newer version which takes reviewer comments into consideration. What method are you using to send the patches? git send-email? Something other? This particular mailing list rejects HTML-formatted messages, so that could be the culprit if you pasted the patch into your email client. It's a good idea to try sending patches to yourself via "git send-email". If you can get that to work successfully, then they should be accepted by the mailing list when sent via the same mechanism. ```