[PATCH] Trim ending whitespaces in exclude file if needed.

[PATCH] Trim ending whitespaces in exclude file if needed.

[Vasyl']

Signed-off-by: Vasyl' Vavrychuk <vvavrychuk@gmail.com>
---

Hope this can save someone's time debugging git.

 dir.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/dir.c b/dir.c
index d1e5e5e..704914b 100644
--- a/dir.c
+++ b/dir.c
@@ -171,7 +171,15 @@ void add_exclude(const char *string, const char *base,
 		to_exclude = 0;
 		string++;
 	}
+
 	len = strlen(string);
+	if (len && isspace((unsigned char)string[len - 1])) {
+		struct strbuf trim_buf = STRBUF_INIT;
+		strbuf_add(&trim_buf, string, len);
+		strbuf_rtrim(&trim_buf);
+		string = strbuf_detach(&trim_buf, &len);
+	}
+
 	if (len && string[len - 1] == '/') {
 		char *s;
 		x = xmalloc(sizeof(*x) + len);
-- 
1.7.3.1.msysgit.0

Re: [PATCH] Trim ending whitespaces in exclude file if needed.

[Jonathan Nieder]

(+cc: msysgit)

Vasyl' wrote:

> Hope this can save someone's time debugging git.

It sounds like there's a story behind this one.  Could you tell it?
That would help future readers of this code to easily determine
why they shouldn't break it.

> --- a/dir.c
> +++ b/dir.c
> @@ -171,7 +171,15 @@ void add_exclude(const char *string, const char *base,
>  		to_exclude = 0;
>  		string++;
>  	}
> +

Why?

>  	len = strlen(string);
> +	if (len && isspace((unsigned char)string[len - 1])) {

This cast is not needed (see git-compat-util.h).

> +		struct strbuf trim_buf = STRBUF_INIT;
> +		strbuf_add(&trim_buf, string, len);
> +		strbuf_rtrim(&trim_buf);

Missing free(string)?

> +		string = strbuf_detach(&trim_buf, &len);
> +	}
> +
>  	if (len && string[len - 1] == '/') {

Thanks for a clear and pleasant read.
Jonathan

Re: [PATCH] Trim ending whitespaces in exclude file if needed.

[Vasyl']

[-- Attachment #1: Type: text/plain, Size: 1517 bytes --]

Jonathan Nieder <jrnieder@gmail.com> wrote:

> (+cc: msysgit)
>
> Vasyl' wrote:
>
> > Hope this can save someone's time debugging git.
>
> It sounds like there's a story behind this one.  Could you tell it?
> That would help future readers of this code to easily determine
> why they shouldn't break it.
>
I modify either .git/info/exclude or .gitignore by copy-pasting `git
status`. But unfortunetly this adds spacing to ends of lines and ignoring
does not work...

>
> > --- a/dir.c
> > +++ b/dir.c
> > @@ -171,7 +171,15 @@ void add_exclude(const char *string, const char
> *base,
> >               to_exclude = 0;
> >               string++;
> >       }
> > +
>
> Why?
>
> >       len = strlen(string);
> > +     if (len && isspace((unsigned char)string[len - 1])) {
>
> This cast is not needed (see git-compat-util.h).
>
> > +             struct strbuf trim_buf = STRBUF_INIT;
> > +             strbuf_add(&trim_buf, string, len);
> > +             strbuf_rtrim(&trim_buf);
>
> Missing free(string)?
>
I have misunderstood in the first iteration memory managment in the
add_exclude's function code
 struct exclude *x;
char *s;
x = xmalloc(sizeof(*x) + len);
s = (char *)(x+1);
memcpy(s, string, len - 1);
s[len - 1] = '\0';
string = s;
And fix of my patch needs more change and testing. I will do this later.

>
> > +             string = strbuf_detach(&trim_buf, &len);
> > +     }
> > +
> >       if (len && string[len - 1] == '/') {
>
> Thanks for a clear and pleasant read.
>
Thanks for review.

> Jonathan
>

[-- Attachment #2: Type: text/html, Size: 3006 bytes --]

Re: [PATCH] Trim ending whitespaces in exclude file if needed.

[Junio C Hamano]

"Vasyl'" <vvavrychuk@gmail.com> writes:

>> It sounds like there's a story behind this one.  Could you tell it?
>> That would help future readers of this code to easily determine
>> why they shouldn't break it.
>>
> I modify either .git/info/exclude or .gitignore by copy-pasting `git
> status`. But unfortunetly this adds spacing to ends of lines and ignoring
> does not work...

That makes it sound as if you would need a patch to fix your editor or
whatever you are using for copy/paste, rather than git, no?

I do not personally have need to ignore paths whose names end with
whitespaces, and I do not think anybody with such a need is sane, so in
that sense your patch would be less harmful than the purist in me finds
issues in it ;-)

But it changes the documented behaviour that requires documentation
updates, yes?