patchgrep - Patches: patch #3644, --initial-tab and 3 newly...

 
 

You are not allowed to post comments on this tracker with your current authentification level.

patch #3644: --initial-tab and 3 newly colorized items

Submitted by:  None
Submitted on:  Fri 14 Jan 2005 10:43:27 AM UTC  
 
Category: NonePriority: 5 - Normal
Status: DonePrivacy: Public
Assigned to: Charles Levert <charles_levert>Originator Email: -unavailable-
Open/Closed: Closed

(Jump to the original submission Jump to the original submission)

Tue 21 Jun 2005 04:34:09 PM UTC, comment #21:

Reworked version part of CVS commit of revisions 1.106, 1.107, 1.108, 1.109, and 1.110 of "src/grep.c". Also revisions 1.53 and 1.54 of "doc/grep.texi" and revisions 1.30 and 1.31 of "doc/grep.1".

Only patch #3767 was not really integrated into this.

Charles Levert <charles_levert>In charge of this item.
Thu 24 Feb 2005 09:04:39 AM UTC, comment #20:

This latest patch (charles2a0.patch) is a trimmed down version that relies on the following hierarchy of smaller patches to have already been applied:

--> patch #3771
....--> patch #3770
........--> patch #3767
........--> patch #3769
--> patch #3768

The only resulting difference from the previous version is the slightly different solution adopted in patch #3768 to handle its targeted problem.

Charles Levert <charles_levert>In charge of this item.
Wed 23 Feb 2005 05:09:49 AM UTC, comment #19:

This latest patch (charles1ux.patch) adds the modifications to -o/--only-matching that I proposed on the bug-grep mailing list:

1) no context lines with --only-matching;

2) print a line head (if any is specified) for every match with --only-matching;

3) any printed byte count with --only-matching is that of the match itself.

Sadly, since I am backporting all these patches from a more heavily modified personal version of grep, two details were in the previous patch that should not have been there (some "ms.ms_" stuff and a "cap = NULL" line that may have been in older versions of the patch as well). This version of the patch fixes this. If at any point some correct older versions of the patch are requested, I can provide them.

Charles Levert <charles_levert>In charge of this item.
Tue 22 Feb 2005 08:40:10 PM UTC, comment #18:

This latest patch (charles1tx.patch) is now based on cvs version 1.93 (latest) of src/grep.c. It changes the following in this file in function prline(), in chunks of code that the patch already touched anyway.

1) Context lines are no longer scanned again for matches with --color (color_option), since it is already known that they don't contain any. That should save time.

2) All the --ignore-case (match_icase) stuff has been removed since the addition made by cvs version 1.30 of src/search.c (2004-12-06) to function check_multibyte_string() makes it unneeded. All other (*execute)() invocations in src/grep.c already work well without it to find matches in the first place.

3) The --color (color_option) and --only-matching (only_matching) code paths have been merged since they are mostly identical and only need a few if:s to make the difference. This makes less code to maintain and synchronize.

4) A test for "lim-beg" before the (*execute)() has been changed to "lim > beg" and is now systematically performed (since it's the only call that remains). All three removed calls were inconsistently missing it, proving that it's a good idea to merge near-identical code in one place and maintain it only there.

Everything else is the same as in the previous patch.

Charles Levert <charles_levert>In charge of this item.
Mon 21 Feb 2005 03:13:52 PM UTC, comment #17:

This latest patch (charles1sx.patch) contains an obvious simplification over the previous one for the empty "mt" capability case. This is actually a big improvement, since it completely skips the costly calls to (*execute)(), as well as to xmalloc() and tolower() if match_icase, in this eventuality.

The idea just came to me, just like that.

Everything else is the same as in the previous patch.

Charles Levert <charles_levert>In charge of this item.
Mon 21 Feb 2005 02:16:37 PM UTC, comment #16:

Hi, Juri. I am glad to see that the patch finds useful applications that use it for its structuring properties.

As per your request, this latest patch (charles1rx.patch) defines and uses macros to print SGR sequences that check if they are worth printing (if --color is in effect and if the relevant capability is not empty).

I believe the new logic in prline() to be correct after having performed some testing myself, but everyone is hereby encouraged to perform their own testing and audit of this code.

Note that there is no analysis of capability non-empty values so that with GREP_COLORS='ml=31:mt=32', there will still be output like
"\33[31mFOO\33[32mMATCH\33[m\33[K\33[31mBAR\33[m\33[K\n"
although
"\33[31mFOO\33[32mMATCH\33[31mBAR\33[m\n"
would have been enough for these specific capability values, because other non-empty capability values need it that way.

I do not plan to ever add this kind of analysis, not only because of its complexity, but also because there is no way to predict all the extensions to SGR sequences that various terminals might support (to offert more colors, more attributes, etc.).

Everything else is the same as in the previous patch.

Charles Levert <charles_levert>In charge of this item.
Mon 21 Feb 2005 10:42:30 AM UTC, comment #15:

Hi, Charles. Half year ago I modified GNU Emacs grep.el to use grep color markers to get the exact positions of grep matches for highlighting them in Emacs. I just tried your latest patch, and it works very well. But since now it prints more control sequences, it doesn't work nicely with grep.el. I have already tried to adjust grep.el to work with your changes, and I see no problems with your patch. I even managed to use fn, nu, ml and cl to get more precise positions of file names and line numbers, defined not by heuristic rules used currently in grep.el, but by exact positions reported via color marker by your patch.

What I want to ask you is to reduce the number of empty control sequences. For example, currently I have:

(setenv "GREP_COLORS" "mt=01;31:ml=34:cx=36:fn=35:nu=32:se=")

As you see above the separator is set to empty string with "se=". However, it outputs a useless sequence:

^[[m:^[[m^[[K

Could you fix it so that it would be possible to not print such sequences at all when color code was set to empty string?

Juri Linkov <jurta>
Sun 20 Feb 2005 02:31:24 AM UTC, comment #14:

This latest patch (charles1qx.patch) adds two GREP_COLORS capabilities: "ml" and "cx" for whole matching lines and for context lines, respectively. Both default to the empty string, such that a "\33[m" (SGR reset) will be output at the beginning of those lines. It is thus up to the user/packager/distributor to define them to something else as part of a whole consistent color theme (including default foreground and background terminal colors). This is because it is too difficult to come up with good generic settings for these, because of their nature.

The effect of "ml" stays active when "mt" kicks in; i.e., no SGR_END is output right before it. When -o/--only-matching is specified, "ml" is not used at all.

The code does its best to output the final SGR_END of a matching ("ml") or context ("cx") line before the line termination control characters, so that colorizing is segregated to each individual line; this plays nicer with pagers. Note though that if a "\r" is the last character in a match itself ("mt"), right before a "\n", they will be separated by an SGR_END; as a result, "less -R" will by default display an exceptional (colorized) standout (reverse video) "^M" and this will alert the user of the extent of the match, a good thing. Should the final "\n" (or even "\0") in the line be part of the match, an SGR_END will be printed after it; that is just the reality of this specific match. These "mt" behaviors are not new to this version of the patch.

The documentation has been updated to reflect this addition.

Everything else is the same as in the previous patch.

Charles Levert <charles_levert>In charge of this item.
Fri 18 Feb 2005 02:36:16 PM UTC, comment #13:

I will take a look at this. Before going any further, should we carry the discussion there (bug #12019), or should that bug be closed in favor of centralizing all discussions about colorizing here (patch #3644)?

Charles Levert <charles_levert>In charge of this item.
Fri 18 Feb 2005 01:28:34 PM UTC, comment #12:

Charles, are you willing to enhance the patch to implement the wish expressed in bug #12019?

Stepan Kasal <kasal>
Project Member
Fri 28 Jan 2005 12:28:29 AM UTC, comment #11:

This latest patch (charles1ox.patch) replaces the "cl" boolean capability with the opposite-semantics "ne" (no EL) boolean capability, thereby changing the default behavior (now to print an EL to Right ("\33[K") with every SGR_END).

The documentation has been updated to reflect this.

Everything else is the same as in the previous patch.

Charles Levert <charles_levert>In charge of this item.
Thu 27 Jan 2005 11:00:42 AM UTC, comment #10:

Charles, thank you for the demonstration script, it indeed explains why \e[m\e[K is needed after each match.

But why cannot be the `cl' option hardwired?

In other words, I'd make sgr_end="\33[m\ee[K" and no cl capability would be needed. What problem could this cause?

Stepan Kasal <kasal>
Project Member
Wed 26 Jan 2005 08:00:37 PM UTC, comment #9:

I have attached the "sgreol.sh" script to help anyone study and understand the back_color_erase issue with SGR (the "\33[m" stuff) first identified in bug #11022. This is what justifies the "cl" capability for GREP_COLORS.

Charles Levert <charles_levert>In charge of this item.
Mon 24 Jan 2005 04:34:04 AM UTC, comment #8:

This latest patch (charles1mx.patch) adds
colorization to two output styles that
were missed ("-l" and "-c"). The
"--initial-tab" logic is stil not applied
to the "-c" case as it would not add
anything useful (there being no original
file content to be tab-aligned in the
output).

Everything else is the same as in the
previous patch.

Charles Levert <charles_levert>In charge of this item.
Mon 17 Jan 2005 05:52:53 PM UTC, comment #7:

This latest patch (charles1kx.patch) adds
a few minor details to the previous one.
It still covers all files (src/grep.c,
doc/grep.texi, and doc/grep.1).

It prints better warnings to stderr
when GREP_COLORS is ill-formed, doesn't
give a value to a capacity that requires
one, or gives a value to a boolean
capacity. It continues processing
of GREP_COLORS in those two last cases
at no additional cost in code.

It also renames one of its preprocessor
macros and adds a couple comments.

Charles Levert <charles_levert>In charge of this item.
Mon 17 Jan 2005 08:15:03 AM UTC, comment #6:

This latest patch (charles1jx.patch) adds
a few minor details to the previous one.
It still covers all files (src/grep.c,
doc/grep.texi, and doc/grep.1).

It adds a "#define SGR_END sgr_end" macro
so as to uniformize the code with regards
to SGR_ARG and hide the exact implementation.

It prints a warning to stderr when GREP_COLORS
is ill-formed to help the user fix the problem.

The documentation states more explicitly when
the "cl" GREP_COLORS capability is useful:
for terminals with the 'back_color_erase'
('bce') terminfo capability when the chosen
highlight colors affect the background.

Charles Levert <charles_levert>In charge of this item.
Sat 15 Jan 2005 11:21:19 PM UTC, comment #5:

This latest patch (charles1ix.patch) covers all files
(src/grep.c, doc/grep.texi, and doc/grep.1).
It adds support for a boolean "cl" capability in
GREP_COLORS to clear to the end of line each time
a colorized item ends (needed on some terminals).

This obsoletes the patch in bug #11022 by
completely fixing all issues it involves.

Charles Levert <charles_levert>In charge of this item.
Sat 15 Jan 2005 01:17:47 PM UTC, comment #4:

Man page now covers --initial-tab and GREP_COLORS.

Charles Levert <charles_levert>In charge of this item.
Sat 15 Jan 2005 01:15:43 PM UTC, comment #3:

TeXinfo documentation now covers --initial-tab
and GREP_COLORS.

Charles Levert <charles_levert>In charge of this item.
Sat 15 Jan 2005 01:44:45 AM UTC, comment #2:

This latest patch (grep.c.diff.charles1hx) implements
everything in the previous one, plus a new environment
variable GREP_COLORS (plural). The old GREP_COLOR is
still supported but is deprecated and doesn't have
priority. Everything is backward compatible.

GREP_COLORS uses a syntax similar to only the
necessary subset of LS_COLORS. Its parser is much
smaller and simpler, is robust, and is forward
compatible with any future property names (newer
colorized items) that might be defined (they are
just ignored for now if well-formed).

Default (as an example):
GREP_COLORS='mt=01;31:fn=35:nu=32:se=36'

This now completely obsoletes patch #883,
which never made it to CVS or release.

Charles Levert <charles_levert>In charge of this item.
Fri 14 Jan 2005 01:47:28 PM UTC, comment #1:

Missed the "--" separator. Use newer patch.

Charles Levert <charles_levert>In charge of this item.
Fri 14 Jan 2005 10:43:27 AM UTC, original submission:

I also submitted something similar to
the <bug-gnu-utils@gnu.org> mailing
list before I found this site.

This is against CVS version 1.91 of
<src/grep.c>, which is current at the
time I am writing this.

I have added the following two features:

  • A "-T" / "--initial-tab"

option (both invocations are
consistent with GNU diff).
This will make sure that the
first character of a content
line is on a tab stop.
The last separator (":") is
positioned immediately before
that so it remains possible
to tell whitespace at the
beginning of the content
line from whitespace used for
padding. I believe it would
be pointless overkill to try
to follow the current column
of output after printing
a filename in Unicode; my
implementation does something
really simple that works.

  • Three other colors when

"--color" is in effect.
They are for the filename,
the numbers (line number
and octet offset),
and the separator (":").
These cannot be customized
by the user yet, but each
is put in its own variable
so everything is in place
for them to be. (I didn't
want to add a bunch of new
environment variables for
that, and I didn't want to
venture a new mechanism for
specifying colors, possibly
by extending the current
environment variable in a
backward compatible way.
This can be done later
if the patch is accepted
and popular, and if users
actually request it.)

Please consider merging this patch with
the official release.

(If you like the patch but think it
is too big to be accepted without
paperwork, please contact me before
doing anything else like rewriting the
whole thing differently just for the
sake of doing it.)

Anonymous

 

Attached Files
file #8459:  charles2a0.patch added by charles_levert (19kB - text/x-patch - trimmed down unidiff patch for --initial-tab and GREP_COLORS)
file #8448:  charles1ux.patch added by charles_levert (22kB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, with --only-matching changes)
file #8447:  charles1tx.patch added by charles_levert (21kB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, with simplification of existing code)
file #8442:  charles1sx.patch added by charles_levert (20kB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, with obvious simplification)
file #8441:  charles1rx.patch added by charles_levert (20kB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, adding empty cap checks)
file #8431:  charles1qx.patch added by charles_levert (19kB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, adding ml and cx caps)
file #8326:  charles1ox.patch added by charles_levert (18kB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8321:  sgreol.sh added by charles_levert (698B - text/x-sh - Shell script to help study the back_color_erase issue with SGR.)
file #8308:  charles1mx.patch added by charles_levert (17kB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8268:  charles1kx.patch added by charles_levert (17kB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8265:  charles1jx.patch added by charles_levert (16kB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8261:  charles1ix.patch added by charles_levert (16kB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8259:  grep.1.diff.charles1hx added by charles_levert (3kB - application/octet-stream - matching unidiff patch for man page)
file #8258:  grep.texi.diff.charles1hx added by charles_levert (3kB - application/octet-stream - matching unidiff patch for TeXinfo documentation)
file #8257:  grep.c.diff.charles1hx added by charles_levert (7kB - application/octet-stream - unidiff patch that implements --initial-tab, 3.5 newly colorized items, and GREP_COLORS against current CVS)
file #8256:  grep.c.diff.charles1gx added by charles_levert (5kB - application/octet-stream - unidiff patch that implements --initial-tab and 3.5 newly colorized items against current CVS)
file #8255:  grep.c.diff.charles1fx added by None (5kB - application/octet-stream - unidiff patch that implements --initial-tab and 3 newly colorized items against current CVS)

 

Digest:
   patch dependencies.

Digest:
   bug dependencies.

 

CC list is empty

 

Do you think this task is very important?
If so, you can click here to add your encouragement to it.
This task has 0 encouragements so far.

Only logged-in users can vote.

 

Please enter the title of George Orwell's famous dystopian book (it's a date):

 

 

Follow 25 latest changes.

Date Changed By Updated Field Previous Value => Replaced By
Tue 21 Jun 2005 04:34:09 PM UTCcharles_levertStatusNone=>Done
  Assigned toNone=>charles_levert
  Open/ClosedOpen=>Closed
Mon 13 Jun 2005 10:09:57 PM UTCjulianfoadDependenciesRemoved dependancy from patch #3801=>-
Thu 28 Apr 2005 04:18:18 PM UTCcharles_levertDependencies-=>patch #3801 is dependent
Tue 12 Apr 2005 03:09:31 PM UTCjulianfoadDependencies-=>Depends on patch #3771
  Dependencies-=>Depends on patch #3770
  Dependencies-=>Depends on patch #3769
  Dependencies-=>Depends on patch #3768
  Dependencies-=>Depends on patch #3767
Thu 24 Feb 2005 09:04:39 AM UTCcharles_levertAttached File-=>Added charles2a0.patch, #4231
Wed 23 Feb 2005 05:09:49 AM UTCcharles_levertAttached File-=>Added charles1ux.patch, #4220
Tue 22 Feb 2005 08:40:10 PM UTCcharles_levertAttached File-=>Added charles1tx.patch, #4219
Mon 21 Feb 2005 03:13:52 PM UTCcharles_levertAttached File-=>Added charles1sx.patch, #4214
Mon 21 Feb 2005 02:16:37 PM UTCcharles_levertAttached File-=>Added charles1rx.patch, #4213
Sun 20 Feb 2005 02:31:24 AM UTCcharles_levertAttached File-=>Added charles1qx.patch, #4203
Fri 28 Jan 2005 12:28:29 AM UTCcharles_levertAttached File-=>Added charles1ox.patch, #4102
Wed 26 Jan 2005 08:00:37 PM UTCcharles_levertAttached File-=>Added sgreol.sh, #4097
Mon 24 Jan 2005 04:34:04 AM UTCcharles_levertAttached File-=>Added charles1mx.patch, #4084
Mon 17 Jan 2005 05:52:54 PM UTCcharles_levertAttached File-=>Added charles1kx.patch, #4051
Mon 17 Jan 2005 08:15:03 AM UTCcharles_levertAttached File-=>Added charles1jx.patch, #4048
Sat 15 Jan 2005 11:21:19 PM UTCcharles_levertAttached File-=>Added charles1ix.patch, #4044
Sat 15 Jan 2005 01:17:47 PM UTCcharles_levertAttached File-=>Added grep.1.diff.charles1hx, #4042
Sat 15 Jan 2005 01:15:43 PM UTCcharles_levertAttached File-=>Added grep.texi.diff.charles1hx, #4041
Sat 15 Jan 2005 01:44:45 AM UTCcharles_levertAttached File-=>Added grep.c.diff.charles1hx, #4040

Back to the top


Powered by Savane 3.1-cleanup