Hi On Fri, Jul 12, 2013 at 10:23 PM, Adam Borowski <kilob...@angband.pl> wrote: > This is not a bug on our side, but a misdesign in ITU T.416, yet with > all popular terminals supporting these codes, people consider this to > be a bug in Linux. By breaking the design principles behind SGR codes > (gracefully ignoring unsupported ones should not require knowing about > them), 256 colour ones tend to turn blinking on before invoking an > arbitrary unrelated command. > > This commit doesn't add such support, merely skips such codes without > ill effects. > > Signed-off-by: Adam Borowski <kilob...@angband.pl> > --- > drivers/tty/vt/vt.c | 26 ++++++++++++++++++++------ > 1 file changed, 20 insertions(+), 6 deletions(-) > > diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c > index c677829..f7aaa28 100644 > --- a/drivers/tty/vt/vt.c > +++ b/drivers/tty/vt/vt.c > @@ -1300,13 +1300,27 @@ static void csi_m(struct vc_data *vc) > case 27: > vc->vc_reverse = 0; > break; > - case 38: /* ANSI X3.64-1979 (SCO-ish?) > - * Enables underscore, white foreground > - * with white underscore (Linux - use > - * default foreground). > + case 38: > + case 48: /* ITU T.416 > + * Higher colour modes. > + * They break the usual properties of SGR > codes > + * and thus need to be detected and ignored > by > + * hand. Strictly speaking, that standard > also > + * wants : rather than ; as separators, > contrary > + * to ECMA-48, but no one produces such codes > + * and almost no one accepts them. > */ > - vc->vc_color = (vc->vc_def_color & 0x0f) | > (vc->vc_color & 0xf0); > - vc->vc_underline = 1;
You break the old behavior here. _Iff_ this is what you want, then please do that in another commit. Explicitly state that "38" is used for 256color and shouldn't turn on underline+default-col. The SCO-ish behavior is weird, indeed, but breaking it silently is not ok. > + i++; > + if (i > vc->vc_npar) This should be ">=", but the for()-loop does allow your ">". So unless someone fixes the for-loop to use "<" (do a ++vc->vc_npar before it, if it's correct. But blindly doing "<=" is really irritating) I think this is ok. > + break; > + if (vc->vc_par[i] == 5) /* 256 colours */ > + i++; /* ubiquitous */ > + else if (vc->vc_par[i] == 2) /* 24 bit > colours */ > + i += 3; /* extremely > rare */ > + /* Subcommands 3 (CMY) and 4 (CMYK) are so > insane > + * that detecting them is not worth the few > extra > + * bytes of kernel's size. > + */ I can confirm that 38/48 are "parsed" correctly here. Skipping '3' and '4' seems right, too. Haven't seen anyone implementing them. Even '2' seems to be almost unused. Btw., you should put Greg Kroah-Hartman and Andrew Morton on CC. Both are the most likely to pick this up. Regards David > break; > case 39: /* ANSI X3.64-1979 (SCO-ish?) > * Disable underline option. > -- > 1.8.4.rc3 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
