On Sun, Jan 08, 2023 at 08:55:46PM +0000, Nicholas Marriott wrote:
> This is still too big for one go so I think it needs split further. I
> suggest that first we get three diffs with the very easy stuff:
> 
> 1) The two bug fixes
> 
> 2) HPA/VPA/INDN/RIN

You didn't mention '3' :-)

But anyway, that's basically the first diff I sent and the two one-liners.

I've attached that to the end of this mail.

However, I think it's counter-productive to put just this in without the
changes to the function keys, because as soon as people start testing the
patch and setting TERM=xterm, then the first thing they are going to complain
about is that F1 - F4 don't work.

> Some other comments on the larger diff:
> 
> - Why do you change RS_SCROLLBACK_SCREENS?

To more closely mirror xterm behaviour.  The current scrollback buffer is a
tiny fraction of what xterm offers by default.  It's not that important, but
if we're telling people that the console is now like xterm, it's better that
it really is.

> - Wouldn't it be better to make underline match rather than moving bits
> about at runtime? Is that not possible?

I'm not sure what you mean here.

The original underlining code is unchanged by my diff.  I only added double
underline and strikethough.

Are you talking about the rasops code that actually plots the extra underline
pixels?  That's done in the same way as the existing code, so I'm not sure
what you would prefer done differently?

Or is it the part in wscons where the underline flag is moved to bit 0?  That
is a nasty hack that is in the original code.  I could take it out, but that
would mean changing all of the bpp specific rasops code and removing the magic
number 1 and replacing it with WSATTR_UNDERLINE.

That sounds like a lot of churn that could potentially introduce new bugs and
would require a lot of testing.  I prefer to leave the original hack in,
because the rasops code has seen little change for many years, and with the
features that I'm adding I don't think it will need to see much more any time
soon.

> - It seems odd that invisible will still show some elements like the
> underline. I think unless we can support it fully it would be better not to
> support it at all.

This is the correct behaviour.  Xterm does the same thing.  Invisible
characters retain underline, double underline, and strikethrough.

It would actually be easier to implement invisible without the extra elements,
but I put in the extra effort to mimick xterm behaviour :).

Oh, by the way, I tested my idea of creating a bold font by over-printing the
glyph one pixel to the right.  It looked good, and if we used this approach
instead of setting a brighter colour for bold, then we could support bold in
conjunction with all 256 colours instead of just the first eight.

--- dev/wscons/wsemul_vt100_subr.c.orig Mon May 25 06:55:49 2020
+++ dev/wscons/wsemul_vt100_subr.c      Sun Jan  8 18:28:04 2023
@@ -231,7 +231,7 @@
        switch (A3(edp->modif1, edp->modif2, c)) {
        case A3('>', '\0', 'c'): /* DA secondary */
                wsdisplay_emulinput(edp->cbcookie, WSEMUL_VT_ID2,
-                   sizeof(WSEMUL_VT_ID2));
+                   sizeof(WSEMUL_VT_ID2)-1);
                break;
 
        case A3('\0', '\0', 'J'): /* ED selective erase in display */
@@ -461,6 +461,9 @@
                edp->ccol -= min(DEF1_ARG(0), edp->ccol);
                edp->flags &= ~VTFL_LASTCHAR;
                break;
+       case 'G': /* HPA */
+               edp->ccol = min(DEF1_ARG(0)-1, edp->ncols-1);
+               break;
        case 'H': /* CUP */
        case 'f': /* HVP */
                if (edp->flags & VTFL_DECOM)
@@ -502,15 +505,36 @@
                WSEMULOP(rc, edp, &edp->abortstate, erasecols,
                    ERASECOLS(NCOLS - n, n, edp->bkgdattr));
                break;
+       case 'S': /* INDN */
+               wsemul_vt100_scrollup (edp,DEF1_ARG(0));
+               break;
+       case 'T': /* RIN */
+               wsemul_vt100_scrolldown (edp,DEF1_ARG(0));
+               break;
        case 'X': /* ECH erase character */
                n = min(DEF1_ARG(0), COLS_LEFT + 1);
                WSEMULOP(rc, edp, &edp->abortstate, erasecols,
                    ERASECOLS(edp->ccol, n, edp->bkgdattr));
                break;
+       case 'Z': /* KCBT */
+               if (!edp->ccol)
+                       break;
+               if (edp->tabs) {
+                       for (n=edp->ccol-1 ; n>0; n--)
+                               if (edp->tabs[n])
+                                       break;
+               } else {
+                       n=((edp->ccol - 1) & 8);
+               }
+               edp->ccol=n;
+               break;
        case 'c': /* DA primary */
                if (ARG(0) == 0)
                        wsdisplay_emulinput(edp->cbcookie, WSEMUL_VT_ID1,
-                           sizeof(WSEMUL_VT_ID1));
+                           sizeof(WSEMUL_VT_ID1)-1);
+               break;
+       case 'd': /* VPA */
+               edp->crow = min(DEF1_ARG(0)-1, edp->nrows-1);
                break;
        case 'g': /* TBC */
                if (edp->tabs != NULL)

Reply via email to