Subject: Re: bug#79298: patch: full color in windows terminal

From: Eli Zaretskii <[email-masked]>
Date: Mon, 25 Aug 2025 13:50:12 +0300

> Date: Mon, 25 Aug 2025 06:09:37 +0000
> From: Ewan <[email-masked]>
> 
> 
> > merge 79298 79297
> 
> > Such a large contribution will need you to sign a
> > copyright-assignment agreement with the FSF.
> 
> 
> I assumed as much, and have sent off my details in the form request to [email-masked].
> If it's easier for you to just send me the form, that works too!

Thanks, form sent off-list.

> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -299,6 +299,21 @@ AC_ARG_WITH([all],
> > > [with_features=$withval],
> > > [with_features=yes])
> > > 
> > > +AC_ARG_WITH([w32-vt-color],
> > > +[AS_HELP_STRING([--with-w32-vt-color=ARG],
> > > +[use VT sequences for w32 console color (ARG one of: no, 16, 156, 24bit)])],
> > > ...
> > 
> > Why do we need this configure-time switch? why not compile this
> > feature into Emacs unconditionally?
> 
> 
> This work was inherited from someone else, and I had wondered the same...

Hmm... the fact that some of the code is "inherited from someone else"
worries me.  If the code you took from someone else's work is
substantial (more than, say, 15 lines), we will need that other person
to assign the copyright to the FSF, in addition to your assignment.

Can you tell who is that other person and how much of the code he/she
wrote?

> > I think it's sub-optimal to have this conditioned on the build-time
> > conditions. I think users should be able to change the number of
> > supported colors when starting Emacs (using the --color command-line
> > option) and/or from within a running Emacs session by changing the
> > value of a variable. Could you please rework the patch to allow that?
> 
> 
> Will do (just need to figure out where the --color arg gets processed).

It is processed in lisp/startup.el.

> > AFAIK, the ENABLE_VIRTUAL_TERMINAL_PROCESSING is not supported on
> > Windows 9X (which we still strive to support).
> 
> Will it be sufficient to disable the enhanced color modes on start up,
> but still allow W9 users to activate them (at their own peril)?

Yes.

> > > + WriteConsole (cur_screen, p, strlen(p), &r, NULL);
> > > + }
> > > + if (fg >= 8 && fg < 16)
> > > + {
> > > + char p[10];
> > > + snprintf(p, 10, "[%lum", fg - 8 + 90);
> > > + WriteConsole (cur_screen, p, strlen(p), &r, NULL);
> > > + }
> > > + ...
> > 
> > Can we make this code more compact, like by using fewer snprintf calls
> > (since they are all so much alike), and just a single WriteConsole
> > call for each of foreground and background colors?
> 
> 
> More compact, definitely, and we should actually be able to get away with:
> - one WriteConsole call for turning the face on
> - one WriteConsole call for writing the text
> - one WriteConsole call for turning the face off
> 
> > Finally, these changes will need a NEWS entry and suitable
> > additions/changes to the user manual.
> 
> 
> I'll add a NEWS entry.

Thanks.

> I'm not sure what exactly editing the manual entails, but happy to do so.

At least the description of --color in the user manual should be
adjusted: it currently says that it has no effect on Windows.  I think
the way to turn on/off 24-bit colors in a running session should also
be mentioned in the MS-Windows-specific appendix to the user manual.

> I'll make these changes in the next few days and send off a new patch.

TIA