[2572] in BarnOwl Developers
Re: [PATCH 3/4] _owl_fmtext_append_fmtext takes an inclusive range
daemon@ATHENA.MIT.EDU (Nelson Elhage)
Mon Aug 2 21:12:06 2010
From: Nelson Elhage <nelhage@MIT.EDU>
To: David A Benjamin <davidben@mit.edu>
Cc: Anders Kaseorg <andersk@mit.edu>, barnowl-dev@mit.edu
In-Reply-To: <alpine.DEB.1.10.1007280049380.2102@dr-wily.mit.edu>
Date: Mon, 02 Aug 2010 21:11:58 -0400
owl_fmtext_expand_tabs ends with:
+ /* Append anything we've missed. */
+ if (numcopied < in->textlen)
+ _owl_fmtext_append_fmtext(out, in, numcopied, in->textlen - 1);
Unless I'm mistaken, that should be updated to be in->textlen?
Oh, I see you fix it later. Might as well clean that up.
Otherwise, lgtm.
- Nelson
On Wed, 28 Jul 2010 00:52:08 -0400 (EDT), David A Benjamin <davidben@MIT.ED=
U> wrote:
> Eh, it's not that hard. I've updated this patchset and expand-tabs on top=
=20
> of it.
>=20
> http://github.com/davidben/barnowl/commits/master
> http://github.com/davidben/barnowl/commits/expand-tabs
>=20
> The test case commit now adds the tests for=20
> owl_fmtext_truncate_{lines,cols} (the other callers of=20
> _owl_fmtext_append_fmtext), the latter reordered from expand-tabs. And=20
> then the next commit switching to a half-open range.
>=20
> David
>=20
> On Tue, 27 Jul 2010, Nelson Elhage wrote:
>=20
> > =3D=3Dandersk, but this is an improvement if it's too much of a pain to=
fix
> > quickly (I haven't checked how many callers it has, but I can't imagine
> > it being that bad).
> >
> > This whole series
> > Reviewed-by: Nelson Elhage <nelhage@mit.edu>
> >
> > On Sun, 25 Jul 2010 18:32:43 -0400 (EDT), Anders Kaseorg <andersk@MIT.E=
DU> wrote:
> >> On Sun, 25 Jul 2010, David Benjamin wrote:
> >>> - _owl_fmtext_append_fmtext(f, in, 0, in->textlen);
> >>> + _owl_fmtext_append_fmtext(f, in, 0, in->textlen - 1);
> >>
> >> I=E2=80=99d rather fix this by changing _owl_fmtext_append_fmtext to t=
ake a
> >> half-open range, like any sane function should.
> >>
> >> Anders
> >>
> >
