# Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir() 2 messages from 2014-03-15 to 2014-03-16. Participants: Akshay Aurora, Thomas Rast. Thread: https://gitlist.dev/t/36170 ## Akshay Aurora, 2014-03-15 06:44 Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir() Message-ID: URL: https://gitlist.dev/e/CAPGnZZn_Wz%3DLywVevmuXWQX4nO67EKMPazB8jKv-jTZ178%3DHdQ%40mail.gmail.com In-Reply-To: <1394800759-87648-1-git-send-email-akshayaurora@yahoo.com> ``` Forgot to mention, this is one of the microprojects for GSoC this year. Would be great to have some feedback. On Fri, Mar 14, 2014 at 6:09 PM, Akshay Aurora wrote: > I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory() > > Signed-off-by: Akshay Aurora > --- > diff-no-index.c | 9 +++++---- > 1 file changed, 5 insertions(+), 4 deletions(-) > > diff --git a/diff-no-index.c b/diff-no-index.c > index 8e10bff..2a17c9f 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_dir(const char *path, struct string_list *list) > { > DIR *dir; > struct dirent *e; > @@ -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)) > string_list_insert(list, e->d_name); > > closedir(dir); > @@ -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_dir(name1, &p1)) > return -1; > - if (name2 && read_directory(name2, &p2)) { > + if (name2 && read_dir(name2, &p2)) { > string_list_clear(&p1, 0); > return -1; > } > -- > 1.8.5.3 > -- With Thanks & Warm Regards Akshay Aurora iakshay.net ``` ## Thomas Rast, 2014-03-16 11:07 Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir() Message-ID: <87eh22xuzb.fsf@thomasrast.ch> URL: https://gitlist.dev/e/87eh22xuzb.fsf%40thomasrast.ch In-Reply-To: ``` Akshay Aurora writes: > Forgot to mention, this is one of the microprojects for GSoC this > year. Would be great to have some feedback. > > On Fri, Mar 14, 2014 at 6:09 PM, Akshay Aurora wrote: >> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory() >> >> Signed-off-by: Akshay Aurora Hmm, the original mail never made it through to me, and gmane doesn't seem to have it either. What happened here? The headers suggest you used git-send-email, which should avoid these problems. Can you dig up the command and configuration you used to send it (but be careful to not post your password!)? On the patch itself: > Subject: Re: [PATCH] Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot() and rename it to read_dir() The subject line is very long. Aim for 50 characters, but certainly no more than 72. You are also conflating two separate things into one patch. Try to avoid doing that. Furthermore I am unconvinced that renaming a function from read_directory() to read_dir() is a win. What are you trying to improve by the rename? Renames are good if they improve clarity and/or consistency, but read_dir() just risks confusion with readdir() and I cannot see any gain in consistency that would compensate for it. >> I have renamed diff-no-index.c:read_directory() to read_dir() to avoid name collision with dir.c:read_directory() Please stick to the style outlined in SubmittingPatches: The body should provide a meaningful commit message, which: . explains the problem the change tries to solve, iow, what is wrong with the current code without the change. . justifies the way the change solves the problem, iow, why the result with the change is better. . alternate solutions considered but discarded, if any. Describe your changes in imperative mood, e.g. "make xyzzy do frotz" instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy to do frotz", as if you are giving orders to the codebase to change its behaviour. Try to make sure your explanation can be understood without external resources. Instead of giving a URL to a mailing list archive, summarize the relevant points of the discussion. Also, please wrap your commit messages at 72 characters. >> --- >> diff-no-index.c | 9 +++++---- The microproject idea said Rewrite diff-no-index.c:read_directory() to use is_dot_or_dotdot(). Try to find other sites that can use that function. Are there any others? -- Thomas Rast tr@thomasrast.ch ```