Re: [patch #4604] Fix left anchors and -w problems

[Charles Levert]

* On Wednesday 2005-11-09 at 15:50:33 +0000, Julian Foad wrote:
> Charles Levert wrote:
> > <http://savannah.gnu.org/patch/?func=detailitem&item_id=4604>
> > <http://savannah.gnu.org/patch/download.php?item_id=4604&item_file_id=5449>
> 
> >The following set of changes make previously failing tests relative to left
> >anchors (^ and \<) and -w pass.
> 
> Excellent.
> 
> >It should address the issues raised in bug #11579 and patch #1834.
> 
> Yes, I confirm it does.
> 
> >It now makes fmbtest.sh Test #6 (G and E) fail, but isn't it that test that
> >has wrong expectations?  (Test that -{G,E} --color=always prefers earlier
> >pattern matches.  Earlier as in -e/-f order.)
> 
> As far as I can see, the test matches its description, and your log message 
> says that your code is intended to do that (prefer matches with earlier 
> patterns), but grep in fact colours text that matches a later pattern.  
> This indicates a bug.

There are two different meanings to
earlier/first/etc. here and I want to make sure
they are not getting confused:

   -- earlier in the list of patterns

   -- earlier in the data string

The Test #6 logic prefers earlier patterns in
the pattern list.  My patch tries to consider all
patterns equally, possibly skipping computations
when what one of these matches is worst than the
best we have so far.

My patch tries to find the match that's leftmost
in the data string.  The old code and Test #6 put
themselves at the mercy of the first pattern in
the list that matches with regard to the position
of that match in the data string.

I forgot to cite this bug:

   <https://savannah.gnu.org/bugs/?func=detailitem&item_id=8243>

My patch attempts to fix the issue it raises.
This issue conflicts with what Test #6 implies.


> Other than that, my only comment is please could you put a doc string on 
> the typedef or #define for the "execute" functions that states, at the very 
> least, the special meaning of "start_pos == -1".  And/or use a defined 
> constant with a suitable name instead of the "(size_t) -1".

I need the following specific feedback regarding this.

Was choosing to use an offset (integer) for
this the right thing to do?  In particular,
would using a character pointer have been better?
A character pointer would have had the following
properties:

   -- NULL can be used as the special value to
      indicate that a non-exact match is desired.
      It is easier to test for NULL, both in source
      code and in compiled machine language.

   -- When comes the time to address the full
      64-bit support issue, a chararacter pointer
      (i.e., an address) may more automatically
      be 64-bit than any integer type.  Less to
      worry about then.  (Or are there hybrid
      "architectures" where it's just the
      opposite?)

On the other end, the data string's extent is
already specified there by an integer length,
not by an end pointer, so it's unclear what's
best to do.

None of these logical values (extent and
start-of-lookup) are made long-lived by this
interface; they are ephemeral.  By that, I mean
that sometimes lenghts and offsets are stored
instead of pointers just in case the data string
might be relocalized by other code; this doesn't
appear to be an issue here for this interface.


Thanks.