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

 
 

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

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

Submitter:  None
Submitted:  Fri 14 Jan 2005 10:43:27 AM UTC
   
 
Category:  None Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  charles_levert Originator Email:  -email is unavailable-
Open/Closed:  Closed

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>
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>
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>
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>
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>
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>
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>
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>
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>
Group 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>
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>
Group 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>
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>
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>
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>
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>
Sat 15 Jan 2005 01:17:47 PM UTC, comment #4: 


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


Charles Levert <charles_levert>
Sat 15 Jan 2005 01:15:43 PM UTC, comment #3: 


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


Charles Levert <charles_levert>
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>
Fri 14 Jan 2005 01:47:28 PM UTC, comment #1: 


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

Charles Levert <charles_levert>
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

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attached Files
file #8459:  charles2a0.patch added by charles_levert (19KiB - text/x-patch - trimmed down unidiff patch for --initial-tab and GREP_COLORS)
file #8448:  charles1ux.patch added by charles_levert (22KiB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, with --only-matching changes)
file #8447:  charles1tx.patch added by charles_levert (21KiB - 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 (20KiB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, with obvious simplification)
file #8441:  charles1rx.patch added by charles_levert (20KiB - text/x-patch - complete unidiff patch for --initial-tab and GREP_COLORS, adding empty cap checks)
file #8431:  charles1qx.patch added by charles_levert (19KiB - 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 (18KiB - 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 (17KiB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8268:  charles1kx.patch added by charles_levert (17KiB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8265:  charles1jx.patch added by charles_levert (16KiB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8261:  charles1ix.patch added by charles_levert (16KiB - text/x-patch - complete revised unidiff patch for --initial-tab and GREP_COLORS)
file #8259:  grep.1.diff.charles1hx added by charles_levert (3KiB - application/octet-stream - matching unidiff patch for man page)
file #8258:  grep.texi.diff.charles1hx added by charles_levert (3KiB - application/octet-stream - matching unidiff patch for TeXinfo documentation)
file #8257:  grep.c.diff.charles1hx added by charles_levert (7KiB - 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 (5KiB - 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 (5KiB - application/octet-stream - unidiff patch that implements --initial-tab and 3 newly colorized items against current CVS)

 

CC list is empty

 

There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

 

Follow 25 latest changes.

Date Changed by Updated Field Previous Value => Replaced by
2005-06-21 charles_levert StatusNone Done
    Assigned toNone charles_levert
    Open/ClosedOpen Closed
2005-06-13 julianfoad DependenciesRemoved dependency from patch #3801 -
2005-04-28 charles_levert Dependencies- patch #3801 is dependent
2005-04-28 charles_levert Dependencies- bugs #11022 is dependent
2005-04-12 julianfoad Dependencies- Depends on patch #3771
    Dependencies- Depends on patch #3770
    Dependencies- Depends on patch #3769
    Dependencies- Depends on patch #3768
    Dependencies- Depends on patch #3767
2005-02-24 charles_levert Attached File- Added charles2a0.patch, #4231
2005-02-23 charles_levert Attached File- Added charles1ux.patch, #4220
2005-02-22 charles_levert Attached File- Added charles1tx.patch, #4219
2005-02-21 charles_levert Attached File- Added charles1sx.patch, #4214
2005-02-21 charles_levert Attached File- Added charles1rx.patch, #4213
2005-02-20 charles_levert Attached File- Added charles1qx.patch, #4203
2005-02-18 kasal Dependencies- bugs #12019 is dependent
2005-01-28 charles_levert Attached File- Added charles1ox.patch, #4102
2005-01-26 charles_levert Attached File- Added sgreol.sh, #4097
2005-01-24 charles_levert Attached File- Added charles1mx.patch, #4084
2005-01-17 charles_levert Attached File- Added charles1kx.patch, #4051
2005-01-17 charles_levert Attached File- Added charles1jx.patch, #4048
2005-01-15 charles_levert Attached File- Added charles1ix.patch, #4044
2005-01-15 charles_levert Attached File- Added grep.1.diff.charles1hx, #4042

Back to the top

Powered by Savane 3.13-f8d8.
Corresponding source code