threads / patch / 36217

v2, 2 parts[GSoC] diff: rename read_directory()

Subject: [PATCH v2 1/2] [GSoC] diff: rename read_directory()

## tl;dr

4 messages between Mar 19, 2014 and Mar 19, 2014. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Brian Bourn· Mar 19, 2014, 06:29 UTC · lore
From: Brian Bourn <ba.bourn@gmail.com>

It is 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 in diff-no-index.c. As a preparatory step, rename the local read_directory() to avoid the collision

Signed-off-by: Brian Bourn <ba.bourn@gmail.com>
---
Part 1 of my GSoC submission
 diff-no-index.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to diff-no-index.c +3 −3
diff --git a/diff-no-index.c b/diff-no-index.c
index 8e10bff..ec51106 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -16,7 +16,7 @@
 #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 +107,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
Brian Bourn· Mar 19, 2014, 06:29 UTC · re: Brian Bourn · lore

[PATCH v2 2/2] [GSoC] diff:use is_dot_or_dotdot() in code

From: Brian Bourn <ba.bourn@gmail.com>
Subject: replace manual "."/".." check with is_dot_or_dotdot()
Signed-off-by: Brian Bourn <ba.bourn@gmail.com>
---
Part 2 of my GSoC submission where the actual change is made
 diff-no-index.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Show changes to diff-no-index.c +2 −1
diff --git a/diff-no-index.c b/diff-no-index.c
index ec51106..c554691 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -15,6 +15,7 @@
 #include "log-tree.h"
 #include "builtin.h"
 #include "string-list.h"
+#include "dir.h"
 
 static int read_directory_contents(const char *path, struct string_list *list)
 {
@@ -25,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
Eric Sunshine· Mar 19, 2014, 08:21 UTC · re: Brian Bourn · lore

Re: [PATCH v2 2/2] [GSoC] diff:use is_dot_or_dotdot() in code

On Wed, Mar 19, 2014 at 2:29 AM, Brian Bourn <ba.bourn@gmail.com> wrote:
> Subject: diff:use is_dot_or_dotdot() in code
Wrong subject. See below.
> From: Brian Bourn <ba.bourn@gmail.com>

Drop this. "git am" will grab your name and address automatically from the email header when applying the patch.

> Subject: replace manual "."/".." check with is_dot_or_dotdot()

This should be the actual subject of this email; and the old, less informative subject, which is still present, should be dropped. Stated differently, make this text the first line of your commit message. "git format-patch" will automatically extract that line as the email's Subject:, and "git am" will convert the Subject: back to the first line of the commit message (without the "Subject:" prefix) when applying the patch.

Other than that, the patch looks fine.
Show 29 quoted lines
> Signed-off-by: Brian Bourn <ba.bourn@gmail.com>
> ---
> Part 2 of my GSoC submission where the actual change is made
>  diff-no-index.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index ec51106..c554691 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -15,6 +15,7 @@
>  #include "log-tree.h"
>  #include "builtin.h"
>  #include "string-list.h"
> +#include "dir.h"
>
>  static int read_directory_contents(const char *path, struct string_list *list)
>  {
> @@ -25,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
Eric Sunshine· Mar 19, 2014, 08:12 UTC · re: Brian Bourn · lore

Re: [PATCH v2 1/2] [GSoC] diff: rename read_directory()

On Wed, Mar 19, 2014 at 2:29 AM, Brian Bourn <ba.bourn@gmail.com> wrote:
> Subject: diff: rename read_directory()
I think you mean "diff-no-index" rather than "diff".
> From: Brian Bourn <ba.bourn@gmail.com>

Unless this is intentionally different from the address from which you sent the email, you shouldn't include it here. "git am" will automatically pick up your name and address directly from the email header when applying your patch.

Show 6 quoted lines
> It is 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
> in diff-no-index.c. As a preparatory step, rename the local
> read_directory() to avoid the collision
Better. Add a period at the end.
Other than these minor points, the patch looks good.
Show 33 quoted lines
> Signed-off-by: Brian Bourn <ba.bourn@gmail.com>
> ---
> Part 1 of my GSoC submission
>  diff-no-index.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/diff-no-index.c b/diff-no-index.c
> index 8e10bff..ec51106 100644
> --- a/diff-no-index.c
> +++ b/diff-no-index.c
> @@ -16,7 +16,7 @@
>  #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 +107,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

← back to recent threads