Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi Jean, At 2022-07-04T14:18:11+0200, Jean Delvare wrote: > Hi Branden, > > On Tue, 2 Oct 2018 03:34:22 -0400, G. Branden Robinson wrote: > > Thanks for your review of these patches; I think we had a good meeting > > of the minds. Just needs that final push. > > It's been a while, I know. Indeed it has! > As part of SUSE's Hack Week 21, I decided to exhume this old patch > series of yours which has been in the back of my mind for the last 4 > years. I could never get over the idea that all the work we both put > into it would go to waste. I agree. I have not forgotten about this; I simply had a couple of international moves, changed jobs, and got even more deeply engrossed in groff development. :) > The project's page is here: > https://hackweek.opensuse.org/21/projects/update-quilts-manual-page > > What I've done so far: > * Rebase the whole series on top of the current version of quilt's > manual page. There were lots of context changes due to additions and > fixes that were made to the manual page over the last 4 years. > * Update individual patches to propagate the changes to the added > paragraphs, so that the changes are consistent and complete for the > current version of the manual page. Sounds good! > Now I'm reaching the next step of the process, which is to go through > my own review and apply the suggested corrections where applicable. > Unfortunately, I see that there were a lot of questions of mine which > were left unanswered. As I do not have your deep knowledge of the roff > language, macros and good practices, I don't feel good enforcing my > views. A bit of good news is that I've managed to learn a few more things over the past four years. So I may now have ideas about how to solve certain problems that I didn't back then. > On the other hand, there are a few changes you proposed which clearly > did not make sense or were confusing, so it also does not feel right > to apply your patches without cleaning them up first. Perfectly fair. > Therefore, if you are still around, still have some interest in the > project, and have a few spare cycles you could allocate for this task, > I would really appreciate if you could go through the questions I > asked back then, and enlighten me. It turns out I never even archived the messages, so I can easily go through them from my (rather bloated) inbox and see if we can get some photons flowing. I'll start on that momentarily. Regards, Branden signature.asc Description: PGP signature ___ Quilt-dev mailing list Quilt-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/quilt-dev
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi Branden, On Tue, 2 Oct 2018 03:34:22 -0400, G. Branden Robinson wrote: > Thanks for your review of these patches; I think we had a good meeting > of the minds. Just needs that final push. It's been a while, I know. As part of SUSE's Hack Week 21, I decided to exhume this old patch series of yours which has been in the back of my mind for the last 4 years. I could never get over the idea that all the work we both put into it would go to waste. The project's page is here: https://hackweek.opensuse.org/21/projects/update-quilts-manual-page What I've done so far: * Rebase the whole series on top of the current version of quilt's manual page. There were lots of context changes due to additions and fixes that were made to the manual page over the last 4 years. * Update individual patches to propagate the changes to the added paragraphs, so that the changes are consistent and complete for the current version of the manual page. Now I'm reaching the next step of the process, which is to go through my own review and apply the suggested corrections where applicable. Unfortunately, I see that there were a lot of questions of mine which were left unanswered. As I do not have your deep knowledge of the roff language, macros and good practices, I don't feel good enforcing my views. On the other hand, there are a few changes you proposed which clearly did not make sense or were confusing, so it also does not feel right to apply your patches without cleaning them up first. Therefore, if you are still around, still have some interest in the project, and have a few spare cycles you could allocate for this task, I would really appreciate if you could go through the questions I asked back then, and enlighten me. Thanks in advance, -- Jean Delvare SUSE L3 Support ___ Quilt-dev mailing list Quilt-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/quilt-dev
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
At 2018-09-17T17:36:23+0200, Jean Delvare wrote: > Hi Branden, Hi Jean! > > On Fri, 2018-06-29 at 09:13 -0400, G. Branden Robinson wrote: > > Thanks for the in-depth review of my monstrous patch sequence! > > > > I'll reply inline for this one but mainly I wanted to ack your review > > and let you know that I'll be getting back to these soon but probably > > not immediately. > > I don't want to sound pushy but do you have any plans to submit an > updated patch series? I would like to release a new version of quilt > "soon" and would really appreciate if all the work you and me have put > into this would make it into that new version. Hi Jean! I am wrapping up a job this week before I relocate (internationally) for work next month. There is a chance I'll be able to put some time into this next week but after that the odds look slimmer because I'll be moving house. Assuming the work visa comes through... Thanks for the reminder. I will try to keep you posted, but if you don't hear from me by October 12th, assume I've fallen off the edge of the world until December or so. Thanks for your review of these patches; I think we had a good meeting of the minds. Just needs that final push. Regards, Branden signature.asc Description: PGP signature ___ Quilt-dev mailing list Quilt-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/quilt-dev
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi Branden, On Fri, 2018-06-29 at 09:13 -0400, G. Branden Robinson wrote: > Thanks for the in-depth review of my monstrous patch sequence! > > I'll reply inline for this one but mainly I wanted to ack your review > and let you know that I'll be getting back to these soon but probably > not immediately. I don't want to sound pushy but do you have any plans to submit an updated patch series? I would like to release a new version of quilt "soon" and would really appreciate if all the work you and me have put into this would make it into that new version. Thanks, -- Jean Delvare SUSE L3 Support ___ Quilt-dev mailing list Quilt-dev@nongnu.org https://lists.nongnu.org/mailman/listinfo/quilt-dev
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi Branden,
On Fri, 29 Jun 2018 09:13:02 -0400, G. Branden Robinson wrote:
> At 2018-06-26T19:00:48+0200, Jean Delvare wrote:
> > > +'\\" t
> >
> > A comment might be in order? Within a week I'll have forgotten the
> > meaning of this line.
> >
> > How portable is this construct?
>
> Well, if we precede it with a comment, it probably gets less portable.
> No fooling. :-|
>
> Various man(1) implementations use this as a hint to decide which
> preprocessors to run.
>
> groff_man(7) says:
>
>If a preprocessor like tbl or eqn is needed, it has become common
>to make the first line of the man page look like this:
>
> '\" word
>
>Note the single space character after the double quote. word
>consists of letters for the needed preprocessors: ‘e’ for eqn,
>‘r’ for refer, and ‘t’ for tbl. Modern implementations of the
>man program read this first line and automatically call the right
>preprocessor(s).
>
> So if the implementation is dumb, it looks _only_ at the first line and
> preceding it with a comment line defeats the purpose.
Adding a comment explaining the purpose of it *after* would be an
option then?
> > > (...)
> > > .I quilt
> > > -uses its predefined color set in order to be more
> > > -comprehensible when distiguishing various types of patches, e.g.\\&
> > > -applied/unapplied, failed, etc.
> > > +which ANSI escape sequences to associate with an output context,
> > > +overriding the defaults.
> >
> > This description does not mention the purpose of the escape sequences.
> > Maybe "with ANSI color escape sequences" would make it clearer?
>
> That omission was deliberate, though perhaps I could make it clearer
> why. You're passing through the escape sequences uninterpreted so
> you're not limited to color changes. You could stick a "1;" into one of
> these variables to turn on boldface, for example.
That's not allowed. The variable is named QUILT_COLORS for a reason.
Anyone abusing it for non-color settings will be prosecuted! :-D
> > > .IP
> > > -To override one or more color settings, set the
> > > +To override one or more settings, set
> >
> > And here again your remove the word "color", while I think it was
> > valuable.
>
> Right. And what I'm saying is that passing these escape sequences makes
> more rendition changes than just color available. QUILT_COLORS is
> unfortunately named, unless you want to add code to filter out SGR
> values that _aren't_ color changes. And I would recommend against that.
Bah, you ruined my attempt at a joke above by anticipating it. I'll
remember that, you will not get away with it! ;-)
> I did some experimenting and things like bold ("1;") and "standout"
> (reverse video, "7;") worked perfectly. In fact I added them to the
> in-page example, with explanatory comments.
I noticed, and approve.
> Another advantage of thinking more expansively is that you achieve
> broader portability and accessibility for people who are using
> monochrome displays, have a defect in color vision, or simply prefer to
> use alternative graphic renditions instead of or in addition to color
> changes.
>
> I doubt you're opposed to the goal; my challenge is to find wording that
> communicates the traditional utility of this variable to your audience.
> :)
My point is that I believe that over 90% of people who would use
QUILT_COLORS will do to actually change the colors. I am worried that
just talking about "ANSI sequences" will lose most of the audience,
even if it is technically correct. So I'd rather mention colors and
then mention other attributes, than not mention either.
Believe it or not, I have myself been working on the topic of ANSI
escape sequences, both colors and attributes, back in year 2000. At the
time, I was in an engineering school with a mixed computing
environment, and had to struggle with the inconsistent ANSI escape
sequence support across the various systems I had to use (AIX, SunOS,
Windows NT, plus Linux at home.) I still have an archive of my work
back then, although I removed it from my home page long ago as I doubt
the results are still useful or even relevant today. A trace of it is
still visible publicly though, in the documentation of the
Term::ANSIColor perl module [1].
> > patch_offs is missing from the list, but that's not your fault, it was
> > already missing from the manual page before.
>
> Okay. I'm noting that to fix it in the next cycle. Thanks for the
> catch!
>
> > > +series_app series applied patch names 32 (green)
> > > +series_top series top patch name 33 (yellow)
> > > +series_una series unapplied patch names 0 (none)
> >
> > series_app, series_top and series_una are also used by "quilt patches".
>
> I'll have to get creative; that table is already pushing the 80-column
> limit. :)
I know. This is why I am telling you that they are missing, but not how
to add them. Because I have no freakin'
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi, Jean!
Thanks for the in-depth review of my monstrous patch sequence!
I'll reply inline for this one but mainly I wanted to ack your review
and let you know that I'll be getting back to these soon but probably
not immediately.
At 2018-06-26T19:00:48+0200, Jean Delvare wrote:
> > Add a hint to the formatter (man) to call the tbl preprocessor render
> > the table.
>
> Missing "to" in above sentence?
Yup--thanks!
> > +'\\" t
>
> A comment might be in order? Within a week I'll have forgotten the
> meaning of this line.
>
> How portable is this construct?
Well, if we precede it with a comment, it probably gets less portable.
No fooling. :-|
Various man(1) implementations use this as a hint to decide which
preprocessors to run.
groff_man(7) says:
If a preprocessor like tbl or eqn is needed, it has become common
to make the first line of the man page look like this:
'\" word
Note the single space character after the double quote. word
consists of letters for the needed preprocessors: ‘e’ for eqn,
‘r’ for refer, and ‘t’ for tbl. Modern implementations of the
man program read this first line and automatically call the right
preprocessor(s).
So if the implementation is dumb, it looks _only_ at the first line and
preceding it with a comment line defeats the purpose.
> > .TP
> > .I QUILT_COLORS
> > -By default,
> > +is a sequence of definitions that direct
>
> As mentioned before, I don't like this "structure".
Acknowledged. I'll undo it here and in the other patches.
> > .I quilt
> > -uses its predefined color set in order to be more
> > -comprehensible when distiguishing various types of patches, e.g.\\&
> > -applied/unapplied, failed, etc.
> > +which ANSI escape sequences to associate with an output context,
> > +overriding the defaults.
>
> This description does not mention the purpose of the escape sequences.
> Maybe "with ANSI color escape sequences" would make it clearer?
That omission was deliberate, though perhaps I could make it clearer
why. You're passing through the escape sequences uninterpreted so
you're not limited to color changes. You could stick a "1;" into one of
these variables to turn on boldface, for example.
> > .IP
> > -To override one or more color settings, set the
> > +To override one or more settings, set
>
> And here again your remove the word "color", while I think it was
> valuable.
Right. And what I'm saying is that passing these escape sequences makes
more rendition changes than just color available. QUILT_COLORS is
unfortunately named, unless you want to add code to filter out SGR
values that _aren't_ color changes. And I would recommend against that.
I did some experimenting and things like bold ("1;") and "standout"
(reverse video, "7;") worked perfectly. In fact I added them to the
in-page example, with explanatory comments.
Another advantage of thinking more expansively is that you achieve
broader portability and accessibility for people who are using
monochrome displays, have a defect in color vision, or simply prefer to
use alternative graphic renditions instead of or in addition to color
changes.
I doubt you're opposed to the goal; my challenge is to find wording that
communicates the traditional utility of this variable to your audience.
:)
> patch_offs is missing from the list, but that's not your fault, it was
> already missing from the manual page before.
Okay. I'm noting that to fix it in the next cycle. Thanks for the
catch!
> > +series_app series applied patch names 32 (green)
> > +series_top series top patch name 33 (yellow)
> > +series_una series unapplied patch names 0 (none)
>
> series_app, series_top and series_una are also used by "quilt patches".
I'll have to get creative; that table is already pushing the 80-column
limit. :)
> > +.TE
> > +.IP
> > +The special
> > +.I format-name
> > +\\[lq]clear\\[rq] is used to turn off special graphics renditions and
> > +return to the terminal defaults.
> > +Changing its definition should
> > +.I not
>
> Is it really worth an emphasis?
If people fiddle with that, they will find their display hideous, and
will probably kill the terminal window in disgust. Honestly, I would
un-document this entirely. I've never heard of a terminal or emulator
that supported ANSI SGR sequences that managed to screw up SGR 0. I
could ask Thomas Dickey; as ncurses and xterm maintainer and a vigorous
monitor of portability issues, he would know better than I. I pay
attention to this sort of thing but he's a true expert.
> > +.UR https://\\:www.ecma\\-international.org/\\:publications/\\
> > +\\:standards/\\:Ecma\\-048.htm
>
> Would seem more simple to keep it on a single line. There is no hard
> limit to the line length.
Fair enough. A lot of people don't know that *roff supports
syntactically transparent newline escapes, like C compilers always have.
(It shouldn't be a surprise, the same people at Bell Labs brought
Re: [Quilt-dev] [patch 26/26] Rewrite discussion of QUILT_COLORS configuration variable.
Hi Branden, On Sat, 16 Jun 2018 12:22:58 -0400, g.branden.robin...@gmail.com wrote: > These are ANSI escape sequences as defined by ECMA-48; recast the entire > discussion in light of that fact. > > Condense the many tagged paragraphs with a templated discussion of > defaults into a table. > > Sort the QUILT_COLORS format names into alphabetical order. > > Add a hint to the formatter (man) to call the tbl preprocessor render > the table. Missing "to" in above sentence? > > Expand the example to be more demonstrative. > > Add pointers to the ECMA-48 standard document and the console_codes > section 4 man page (from Michael Kerrisk's man-pages project, widely > available) to the See Also section. > > Index: quilt/doc/quilt.1.in > === > --- quilt.orig/doc/quilt.1.in > +++ quilt/doc/quilt.1.in > @@ -1,3 +1,4 @@ > +'\\" t A comment might be in order? Within a week I'll have forgotten the meaning of this line. How portable is this construct? > .\\" Created by Martin Quinson from the tex documentation > .\\" > .TH quilt 1 "Dec 17, 2013" "quilt" > @@ -403,103 +404,89 @@ If none of these variables is set, \\[lq > An empty value indicates that no pager should be used. > .TP > .I QUILT_COLORS > -By default, > +is a sequence of definitions that direct As mentioned before, I don't like this "structure". > .I quilt > -uses its predefined color set in order to be more > -comprehensible when distiguishing various types of patches, e.g.\\& > -applied/unapplied, failed, etc. > +which ANSI escape sequences to associate with an output context, > +overriding the defaults. This description does not mention the purpose of the escape sequences. Maybe "with ANSI color escape sequences" would make it clearer? > .IP > -To override one or more color settings, set the > +To override one or more settings, set And here again your remove the word "color", while I think it was valuable. > .I QUILT_COLORS > -variable in following syntax - colon (:) separated list of elements, > -each being of the form =[; -color>] > +to a colon-separated list of elements, > +each of the form > +.RI \\[lq] format-name\\c > +.BI = digit-sequence\\c > +.RB [ ; ...]\\[rq]. > .IP > -Format names with their respective default values are listed below, > -along with their usage(s). > -Color codes(values) are standard bash coloring escape codes. > -See more at http://tldp.org/LDP/abs/html/colorizing.html#AEN20229 > -.RS 4 > -.TP > -.B diff_hdr > -Used in \\[lq]quilt diff\\[rq] to color the index line. > -Defaults to 32 (green). > -.TP > -.B diff_add > -Used in \\[lq]quilt diff\\[rq] to color added lines. > -Defaults to 36 (azure). > -.TP > -.B diff_mod > -Used in \\[lq]quilt diff\\[rq] to color modified lines. > -Defaults to 35 (purple). > -.TP > -.B diff_rem > -Used in \\[lq]quilt diff\\[rq] to color removed lines. > -Defaults to 35 (purple). > -.TP > -.B diff_hunk > -Used in \\[lq]quilt diff\\[rq] to color hunk header. > -Defaults to 33 (brown/orange). > -.TP > -.B diff_ctx > -Used in \\[lq]quilt diff\\[rq] to color the text after end of hunk > -header (\\[lq]diff \\-\\-show\\-c\\-function\\[rq] generates this). > -Defaults to 35 (purple). > -.TP > -.B diff_cctx > -Used in \\[lq]quilt diff\\[rq] to color the 15-asterisk sequence before > -or after a hunk. > -Defaults to 33 (brown/orange). > -.TP > -.B patch_fuzz > -Used in \\[lq]quilt push\\[rq] to color the patch fuzz information. > -Defaults to 35 (purple). > -.TP > -.B patch_fail > -Used in \\[lq]quilt push\\[rq] to color the fail message. > -Defaults to 31 (red). > -.TP > -.B series_app > -Used in \\[lq]quilt series\\[rq] and \\[lq]quilt patches\\[rq] to color > -the applied patch names. > -Defaults to 32 (green). > -.TP > -.B series_top > -Used in \\[lq]quilt series\\[rq] and \\[lq]quilt patches\\[rq] to color > -the top patch name. > -Defaults to 33 (brown/orange). > -.TP > -.B series_una > -Used in \\[lq]quilt series\\[rq] and \\[lq]quilt patches\\[rq] to color > -unapplied patch names. > -Defaults to 0 (no special color). > -.RE > -.RS 4 > -.PP > -In addition, the > -.B clear > -format name is used to turn off special > -coloring. > -Its value is 0; it is not advised to modify it. > -.PP > -The content of > -.I QUILT_COLORS > -supersedes default values. > -So the value \\[lq]diff_hdr=35;44\\[rq] will get you the > -.I diff > -headers in magenta over blue instead of the default green over unchanged > -background. > -For that, add the following content to > +Each > +.I format-name > +with its respective default > +.I digit-sequence > +value is listed below, > +along with an explanation of where it is used. > +Each > +.I digit-sequence > +should be a SGR (Select Graphic Rendition) value supported by your > +terminal. > +The standardized SGR values were specified by ANSI and incorporated > +into ISO-6429 and ECMA-48 (\\[sc]8.3.117). > +The colors have standard names but their values were not defined within > +a
