threads / patch / 25456

patchTrim ending whitespaces in exclude file if needed.

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

## tl;dr

4 messages between Oct 15, 2010 and Oct 18, 2010. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Vasyl'· Oct 15, 2010, 22:41 UTC · lore

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(-)

Show changes to dir.c +8 −0

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

Jonathan Nieder· Oct 17, 2010, 02:41 UTC · re: Vasyl' · lore

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

(+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.

Show 7 quoted lines

> --- 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

Vasyl'· Oct 17, 2010, 09:29 UTC · re: Jonathan Nieder · lore

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

Jonathan Nieder <jrnieder@gmail.com> wrote:

Show 10 quoted lines

> (+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...

Show 23 quoted lines

>
> > --- 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.

Show 8 quoted lines

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

Thanks for review.

> Jonathan
>

Junio C Hamano· Oct 18, 2010, 21:54 UTC · re: Vasyl' · lore

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

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

Show 7 quoted lines

>> 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?