On Sun, Apr 03, 2011 at 04:23:07PM +0300, Martynas Venckus wrote:
> > Comments?
>
> Thanks for the detailed report.
:-)
I don't know how but I somehow missed this message in my
inbox. I only noticed it this past Sunday.
> However, your diff is wrong.
I didn't think it was the optimal solution.
> > Index: vi/vs_smap.c
> > ===================================================================
> > RCS file: /cvs/obsd/src/usr.bin/vi/vi/vs_smap.c,v
> > retrieving revision 1.7
> > diff -u -p vi/vs_smap.c
> > --- vi/vs_smap.c 27 Oct 2009 23:59:49 -0000 1.7
> > +++ vi/vs_smap.c 8 Dec 2010 06:22:09 -0000
> > @@ -224,6 +224,17 @@ top: HMAP->lno = lno;
> > HMAP->coff = 0;
> > HMAP->soff = 1;
> > }
> > + else {
>
> This is not the right place to fix this, since it would affect the
> other 6 calls with OOBLNO in a bad way.
>
> > + /*
> > + * If number of lines HMAP->lno (top line) spans
> > + * changed due to, say reformatting, and now is
> > + * fewer than HMAP->soff, reset so the line is
> > + * redrawn at the top of the screen.
> > + */
>
> This should additionally check if the current line is HMAP->lno
> (top line); otherwise you're changing behaviour in a strange way,
> the top line will be forced to be on top. (Try it.)
I don't understand what you mean.
> > + cnt = vs_screens(sp, HMAP->lno, NULL);
>
> This could use the cached value (cno), it would be faster for larger
> lines.
>
> > + if (cnt < HMAP->soff)
> > + HMAP->soff = 1;
>
> This should actually be cnt, not 1--you're forcing the top to be
> at the beginning of the line; however this is only good for your
> test case (two lines become one). This is bad for longer lines.
It was intended, and I know it isn't the best way but
it was better than the glitch+crash.
> > + }
>
> A similar problem applies for the TMAP case, too.
I don't see it with my patch applied. Maybe I'm missing
steps you follow.
> > /* If we fail, just punt. */
> > for (p = HMAP, cnt = sp->t_rows; --cnt; ++p)
> > if (vs_sm_next(sp, p, p + 1))
>
> Could you verify that the attached diff fixes your problem?
Yes. Thanks! But I see that your patch has been committed
already. Sorry for the lag. As I said, I only saw your
message this past Sunday.
--patrick
> Index: vi/vs_refresh.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/vi/vi/vs_refresh.c,v
> retrieving revision 1.16
> diff -u -r1.16 vs_refresh.c
> --- vi/vs_refresh.c 27 Oct 2009 23:59:49 -0000 1.16
> +++ vi/vs_refresh.c 3 Apr 2011 10:13:47 -0000
> @@ -199,9 +199,18 @@
> else if (F_ISSET(sp, SC_SCR_CENTER)) {
> if (vs_sm_fill(sp, LNO, P_MIDDLE))
> return (1);
> - } else
> + } else {
> + if (LNO == HMAP->lno || LNO == TMAP->lno) {
> + cnt = vs_screens(sp, LNO, &CNO);
> + if (LNO == HMAP->lno && cnt < HMAP->soff)
> + HMAP->soff = cnt;
> + if (LNO == TMAP->lno && cnt > TMAP->soff)
> + TMAP->soff = cnt;
> + }
> +
> if (vs_sm_fill(sp, OOBLNO, P_TOP))
> return (1);
> + }
> F_SET(sp, SC_SCR_REDRAW);
> }
