Re: [PATCH] Fix bug in parse_color that prevented the user from changing the background colors.

From: Timo Hirvonen <tihirvon@gmail.com>
Date: Feb 5, 2008, 18:58 UTC
Message-ID: <20080205205856.76a7cd45.tihirvon@gmail.com>
In-Reply-To: <b6ebd0a50802051045t4949df68u7e405ea618403a31@mail.gmail.com>

"Chris Larson" <clarson@kergoth.com> wrote:

Show 21 quoted lines

> On Feb 5, 2008 11:39 AM, Timo Hirvonen <tihirvon@gmail.com> wrote:
> 
> > Chris Larson <clarson@kergoth.com> wrote:
> >
> > unable to convert the string in its entirety into an integer.
> > > -    if (*name && !*end && i >= -1 && i <= 255)
> > > +    if (*name && i >= -1 && i <= 255)
> > >          return i;
> > >      return -2;
> > >  }
> >
> > My bug, sorry. I should have tested more.
> >
> > I think this new code accepts "7bold" (didn't test).  Maybe you should
> > do something like this instead:
> >
> >        if (*name && (!*end || isspace(*end)) && i >= -1 && i <= 255)
> 
> 
> Indeed, I should have tested more too :)  Just starting to mess with the
> codebase, surprised at how clean it is (though I probably shouldn't be).

OK, did some testing.

The old code accepted attribute before color (bold red). With your patch that "bold" is treaded as fg color -1 and red as bg color, so you need that (!*end || isspace(*end)) test.

Show 6 quoted lines

> > Untested of course.  BTW, your patch is whitespace corrupted.
> >
> 
> Unsurprising... sometimes I really hate gmail :|  Anyone know if there's a
> way to post with gmail without hosing the patch, or should I switch to a
> non-web based solution?

I don't know.  Maybe attachment is the only way.