Re: [PATCH 5/5] ui/console-vc: implement DCH (delete) and ICH (insert) commands

[Roman Penyaev <r.peniaev@gmail.com>]

On Mon, Feb 24, 2025 at 8:25 AM Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> On Sun, Feb 23, 2025 at 6:56 PM Roman Penyaev <r.peniaev@gmail.com> wrote:
> >
> > This patch implements DCH (delete character) and ICH (insert
> > character) commands.
> >
> > DCH - Delete Character:
> >    "As characters are deleted, the remaining characters between the
> >     cursor and right margin move to the left. Character attributes move
> >     with the characters. The terminal adds blank spaces with no visual
> >     character attributes at the right margin. DCH has no effect outside
> >     the scrolling margins" [1].
> >
> > ICH - Insert Character:
> >    "The ICH sequence inserts Pn blank characters with the normal
> >     character attribute. The cursor remains at the beginning of the
> >     blank characters. Text between the cursor and right margin moves to
> >     the right. Characters scrolled past the right margin are lost. ICH
> >     has no effect outside the scrolling margins" [2].
> >
> > Without these commands console is barely usable.
> >
> > [1] https://vt100.net/docs/vt510-rm/DCH.html
> > [1] https://vt100.net/docs/vt510-rm/ICH.html
> >
> > Signed-off-by: Roman Penyaev <r.peniaev@gmail.com>
> > Cc: "Marc-André Lureau" <marcandre.lureau@redhat.com>
> > Cc: qemu-devel@nongnu.org
> > ---
> >  ui/console-vc.c | 108 +++++++++++++++++++++++++++++++++++++++++++++---
> >  1 file changed, 102 insertions(+), 6 deletions(-)
> >
> > diff --git a/ui/console-vc.c b/ui/console-vc.c
> > index 522adc2806c8..bc667897d1bc 100644
> > --- a/ui/console-vc.c
> > +++ b/ui/console-vc.c
> > @@ -598,21 +598,29 @@ static void vc_clear_xy(VCChardev *vc, int x, int y)
> >      vc_update_xy(vc, x, y);
> >  }
> >
> > -static void vc_put_one(VCChardev *vc, int ch)
> > +static void vc_insert_xy(VCChardev *vc, int ch, int x, int y)
> >  {
> >      QemuTextConsole *s = vc->console;
> >      TextCell *c;
> >      int y1;
> > +
> > +    y1 = (s->y_base + y) % s->total_height;
> > +    c = &s->cells[y1 * s->width + x];
> > +    c->ch = ch;
> > +    c->t_attrib = vc->t_attrib;
> > +    vc_update_xy(vc, x, y);
> > +}
> > +
> > +static void vc_put_one(VCChardev *vc, int ch)
> > +{
> > +    QemuTextConsole *s = vc->console;
> > +
> >      if (s->x >= s->width) {
> >          /* line wrap */
> >          s->x = 0;
> >          vc_put_lf(vc);
> >      }
> > -    y1 = (s->y_base + s->y) % s->total_height;
> > -    c = &s->cells[y1 * s->width + s->x];
> > -    c->ch = ch;
> > -    c->t_attrib = vc->t_attrib;
> > -    vc_update_xy(vc, s->x, s->y);
> > +    vc_insert_xy(vc, ch, s->x, s->y);
> >      s->x++;
> >  }
> >
> > @@ -645,6 +653,88 @@ static void vc_set_cursor(VCChardev *vc, int x, int y)
> >      s->y = y;
> >  }
> >
> > +/**
> > + * vc_csi_P() - (DCH) deletes one or more characters from the cursor
> > + * position to the right. As characters are deleted, the remaining
> > + * characters between the cursor and right margin move to the
> > + * left. Character attributes move with the characters.
> > + */
> > +static void vc_csi_P(struct VCChardev *vc, unsigned int nr)
> > +{
> > +    QemuTextConsole *s = vc->console;
> > +    TextCell *c1, *c2;
> > +    unsigned int x1, x2, y;
> > +    unsigned int end, len;
> > +
> > +    if (!nr) {
> > +        nr = 1;
> > +    }
> > +    if (nr > s->width - s->x) {
> > +        nr = s->width - s->x;
> > +        if (!nr) {
> > +            return;
> > +        }
> > +    }
> > +
> > +    x1 = s->x;
> > +    x2 = s->x + nr;
> > +    len = s->width - x2;
> > +    if (len) {
> > +        y = (s->y_base + s->y) % s->total_height;
> > +        c1 = &s->cells[y * s->width + x1];
> > +        c2 = &s->cells[y * s->width + x2];
> > +        memmove(c1, c2, len * sizeof(*c1));
> > +        for (end = x1 + len; x1 < end; x1++) {
> > +            vc_update_xy(vc, x1, s->y);
> > +        }
> > +    }
> > +    /* Clear the rest */
> > +    for (; x1 < s->width; x1++) {
> > +        vc_clear_xy(vc, x1, s->y);
> > +    }
> > +}
> > +
> > +/**
> > + * vc_csi_at() - (ICH) inserts `nr` blank characters with the normal
> > + * character attribute. The cursor remains at the beginning of the
>
> What is "normal character attribute"?
>
> Should you set it to  TEXT_ATTRIBUTES_DEFAULT while inserting blank chars?

Hi,

The wording "default attributes" is more precise, I like it. The
original VT spec
and ECMA-48 are rather vague about exactly what attributes should be set
for "erased state", but various terminal implementations (including vt.c from
the Linux kernel) simply reset the character to the default state. From the
perspective of the existing console-vc.c API, a more correct call is
vt_clear_xt(),
which clears attributes to default rather than inserting a "space"
with vt_put_xy().
I'll fix that. Thanks.

--
Roman