string-match doesn't honor anchor when START is nonzero

string-match doesn't honor anchor when START is nonzero

[Jim Meyering]

In GNU Emacs 20.5.1 (i686-pc-linux-gnu, X toolkit)
 of Fri Dec 10 1999 on ixi.eng.ascend.com
configured using `configure  --prefix=/p/p/emacs-20.5'
------------

The following seems to be at the root of a problem I noticed in Gnus.

Is it expected that this would evaluate to nil?  (it does)

  (string-match "^j" " j" 1)

I expected it to return `1', but the `^' gets in the way.
The documentation for string-match doesn't address this point
as far as I could see.

If returning `nil' is the intended behavior, then there's a bug in
mail-utils.el (rmail-dont-reply-to) because it seems to expect `1' and
fails when it gets the `nil'.  The result is that that function removes
only one of two matching, adjacent addresses.

This illustrates the problem:

  (setq rmail-dont-reply-to-names "f.")
  (load "mail-utils")
  (rmail-dont-reply-to "b, f1, f2, f3, f4, f5, f6")

  Return value: "b, f2, f4, f6"

It should be

  Return value: "b"

Re: string-match doesn't honor anchor when START is nonzero

[Richard Stallman]

    Is it expected that this would evaluate to nil?  (it does)

      (string-match "^j" " j" 1)

That is correct.  The argument 1 means to look for a match starting no
earlier than position 1, but the criterion for a match does not treat
position 1 as the start of a line if it is not one.

    If returning `nil' is the intended behavior, then there's a bug in
    mail-utils.el (rmail-dont-reply-to) because it seems to expect `1' and
    fails when it gets the `nil'.

Could you show which call to string-match is seems to have the problem?
It seems you have tracked this down already; I did a quick scan and
I don't see a bug of this kind.  We could debug it, but if you already
have done so, why not tell us what you know?

Re: string-match doesn't honor anchor when START is nonzero

[Jim Meyering]

Richard Stallman <rms@gnu.org> writes:

|     Is it expected that this would evaluate to nil?  (it does)
|
|       (string-match "^j" " j" 1)
|
| That is correct.  The argument 1 means to look for a match starting no
| earlier than position 1, but the criterion for a match does not treat
| position 1 as the start of a line if it is not one.
|
|     If returning `nil' is the intended behavior, then there's a bug in
|     mail-utils.el (rmail-dont-reply-to) because it seems to expect `1' and
|     fails when it gets the `nil'.
|
| Could you show which call to string-match is seems to have the problem?
| It seems you have tracked this down already; I did a quick scan and
| I don't see a bug of this kind.  We could debug it, but if you already
| have done so, why not tell us what you know?

  This illustrates the problem:

    (setq rmail-dont-reply-to-names "f.")
    (load "mail-utils")
    (rmail-dont-reply-to "b, f1, f2, f3, f4, f5, f6")

    Return value: "b, f2, f4, f6"

  It should be

    Return value: "b"

Here's part of mail-utils.el:
========================================
(defun rmail-dont-reply-to (userids)
  "Returns string of mail addresses USERIDS sans any recipients
that start with matches for `rmail-dont-reply-to-names'.
Usenet paths ending in an element that matches are removed also."
  (if (null rmail-dont-reply-to-names)
      (setq rmail-dont-reply-to-names
	    (concat (if rmail-default-dont-reply-to-names
			(concat rmail-default-dont-reply-to-names "\\|")
		        "")
		    (concat (regexp-quote (user-login-name))
			    "\\>"))))
  (let ((match (concat "\\(^\\|,\\)[ \t\n]*"
		       ;; Can anyone figure out what this is for?
		       ;; Is it an obsolete remnant of another way of
		       ;; handling Foo Bar <foo@machine>?
		       "\\([^,\n]*[!<]\\|\\)"
		       "\\("
			     rmail-dont-reply-to-names
		       "\\|"
		             ;; Include the human name that precedes <foo@bar>.
			     "\\([^\,.<\"]\\|\"[^\"]*\"\\)*"
			     "<\\(" rmail-dont-reply-to-names "\\)"
		       "\\)"))
	(case-fold-search t)
	pos epos)
    (while (setq pos (string-match match userids pos))
      (if (> pos 0) (setq pos (match-beginning 2)))
      (setq epos
	    ;; Delete thru the next comma, plus whitespace after.
	    (if (string-match ",[ \t\n]*" userids (match-end 0))
		(match-end 0)
	      (length userids)))
========================================

This part of the regex

  "\\(^\\|,\\)[ \t\n]*"

is wrong the second time around because we've already advanced past the
comma (2nd string-match above).  That means we end up advancing past a
potentially matching address to the next comma.  Hence, in the example
above, only every other matching address is removed.

Re: string-match doesn't honor anchor when START is nonzero

[Gerd Moellmann]

Jim Meyering <meyering@ascend.com> writes:

> In GNU Emacs 20.5.1 (i686-pc-linux-gnu, X toolkit)
>  of Fri Dec 10 1999 on ixi.eng.ascend.com
> configured using `configure  --prefix=/p/p/emacs-20.5'
> ------------
> 
> The following seems to be at the root of a problem I noticed in Gnus.
> 
> Is it expected that this would evaluate to nil?  (it does)
> 
>   (string-match "^j" " j" 1)
> 
> I expected it to return `1', but the `^' gets in the way.
> The documentation for string-match doesn't address this point
> as far as I could see.
> 

The `^' is a special character in regular expressions, matching
at the beginning of text.

> If returning `nil' is the intended behavior, then there's a bug in
> mail-utils.el (rmail-dont-reply-to) because it seems to expect `1' and
> fails when it gets the `nil'.  The result is that that function removes
> only one of two matching, adjacent addresses.
> 
> This illustrates the problem:
> 
>   (setq rmail-dont-reply-to-names "f.")
>   (load "mail-utils")
>   (rmail-dont-reply-to "b, f1, f2, f3, f4, f5, f6")
> 
>   Return value: "b, f2, f4, f6"
> 

Thanks for the test case.  Does the following patch solve the problem?

Index: mail-utils.el
===================================================================
RCS file: /gd/gnu/cvsroot/emacs/lisp/mail/mail-utils.el,v
retrieving revision 1.44
diff -c -r1.44 mail-utils.el
*** mail-utils.el	1999/08/16 03:14:25	1.44
--- mail-utils.el	2000/01/26 16:06:58
***************
*** 230,254 ****
  	(case-fold-search t)
  	pos epos)
      (while (setq pos (string-match match userids pos))
!       (if (> pos 0) (setq pos (match-beginning 2)))
!       (setq epos
! 	    ;; Delete thru the next comma, plus whitespace after.
! 	    (if (string-match ",[ \t\n]*" userids (match-end 0))
! 		(match-end 0)
! 	      (length userids)))
!       ;; Count the double-quotes since the beginning of the list.
!       ;; Reject this match if it is inside a pair of doublequotes.
!       (let (quote-pos inside-quotes)
! 	(while (and (setq quote-pos (string-match "\"" userids quote-pos))
! 		    (< quote-pos pos))
! 	  (setq quote-pos (1+ quote-pos))
! 	  (setq inside-quotes (not inside-quotes)))
  	(if inside-quotes
  	    ;; Advance to next even-parity quote, and scan from there.
  	    (setq pos (string-match "\"" userids pos))
! 	  (setq userids
! 		(mail-string-delete
! 		 userids pos epos)))))
      ;; get rid of any trailing commas
      (if (setq pos (string-match "[ ,\t\n]*\\'" userids))
  	(setq userids (substring userids 0 pos)))
--- 230,248 ----
  	(case-fold-search t)
  	pos epos)
      (while (setq pos (string-match match userids pos))
!       ;; If there's a match, it starts at the beginning of the string,
!       ;; or with `,'.  We must delete from that position to the
!       ;; end of the user-id which starts at match-beginning 2.
!       (let (inside-quotes quote-pos)
! 	(save-match-data
! 	  (while (and (setq quote-pos (string-match "\"" userids quote-pos))
! 		      (< quote-pos pos))
! 	    (setq quote-pos (1+ quote-pos))
! 	    (setq inside-quotes (not inside-quotes))))
  	(if inside-quotes
  	    ;; Advance to next even-parity quote, and scan from there.
  	    (setq pos (string-match "\"" userids pos))
! 	  (setq userids (replace-match "" nil nil userids)))))
      ;; get rid of any trailing commas
      (if (setq pos (string-match "[ ,\t\n]*\\'" userids))
  	(setq userids (substring userids 0 pos)))
***************
*** 256,261 ****
--- 250,256 ----
      (if (string-match "\\s *" userids)
  	(substring userids (match-end 0))
        userids)))
+ 
  \f
  ;;;###autoload
  (defun mail-fetch-field (field-name &optional last all list)

Re: string-match doesn't honor anchor when START is nonzero

[Jim Meyering]

Gerd Moellmann <gerd@gnu.org> writes:
| > This illustrates the problem:
| >
| >   (setq rmail-dont-reply-to-names "f.")
| >   (load "mail-utils")
| >   (rmail-dont-reply-to "b, f1, f2, f3, f4, f5, f6")
| >
| >   Return value: "b, f2, f4, f6"
| >
|
| Thanks for the test case.  Does the following patch solve the problem?

Thanks for the quick work, but that doesn't quite do it
since it removes only the matching part of the ID.  Instead,
it should remove the entire ID.

Consider this example:
  (rmail-dont-reply-to "b, foo1, foo2, no-match")
With your patch, it evaluates to this:
  bo1o2, no-match

Re: string-match doesn't honor anchor when START is nonzero

[Gerd Moellmann]

Jim Meyering <meyering@ascend.com> writes:

> Gerd Moellmann <gerd@gnu.org> writes:
> | > This illustrates the problem:
> | >
> | >   (setq rmail-dont-reply-to-names "f.")
> | >   (load "mail-utils")
> | >   (rmail-dont-reply-to "b, f1, f2, f3, f4, f5, f6")
> | >
> | >   Return value: "b, f2, f4, f6"
> | >
> |
> | Thanks for the test case.  Does the following patch solve the problem?
> 
> Thanks for the quick work, but that doesn't quite do it
> since it removes only the matching part of the ID.  Instead,
> it should remove the entire ID.
> 
> Consider this example:
>   (rmail-dont-reply-to "b, foo1, foo2, no-match")
> With your patch, it evaluates to this:
>   bo1o2, no-match

Ah, sorry, you're right.  How about this one, applied to the patched
version?



diff -c -r1.1 mail-utils.el
*** mail-utils.el       2000/01/27 12:48:26     1.1
--- mail-utils.el       2000/01/27 12:50:08
***************
*** 226,232 ****
                             ;; Include the human name that precedes <foo@bar>.
                             "\\([^\,.<\"]\\|\"[^\"]*\"\\)*"
                             "<\\(" rmail-dont-reply-to-names "\\)"
!                      "\\)"))
        (case-fold-search t)
        pos epos)
      (while (setq pos (string-match match userids pos))
--- 226,232 ----
                             ;; Include the human name that precedes <foo@bar>.
                             "\\([^\,.<\"]\\|\"[^\"]*\"\\)*"
                             "<\\(" rmail-dont-reply-to-names "\\)"
!                      "\\)[^,]*"))
        (case-fold-search t)
        pos epos)
      (while (setq pos (string-match match userids pos))

Re: string-match doesn't honor anchor when START is nonzero

[Jim Meyering]

Gerd Moellmann <gerd@gnu.org> writes:
| Ah, sorry, you're right.  How about this one, applied to the patched
| version?

Yes.  Now it works.
Thanks again.

mail-utils.el (rmail-dont-reply-to) is still buggy

[Jim Meyering]

Jim Meyering <meyering@ascend.com> writes:
| Gerd Moellmann <gerd@gnu.org> writes:
| | Ah, sorry, you're right.  How about this one, applied to the patched
| | version?
|
| Yes.  Now it works.
| Thanks again.

Actually, it still doesn't work as I'd expect.
In replying (with Gnus) to this message (and others) where
my name was first on the derived To: list, I get this:

  To: , gerd@gnu.org

Here's a small example.  This

  (setq rmail-dont-reply-to-names "b")
  (load "mail-utils")
  (rmail-dont-reply-to "b, f1")

evaluates to this:

  , f1

Obviously, I don't want the leading comma.

Jim

Re: mail-utils.el (rmail-dont-reply-to) is still buggy

[Gerd Moellmann]

Jim Meyering <meyering@ascend.com> writes:

> Obviously, I don't want the leading comma.
> 

This should fix it

diff -c -r1.1 mail-utils.el
*** mail-utils.el       2000/02/12 16:37:16     1.1
--- mail-utils.el       2000/02/12 16:42:07
***************
*** 247,253 ****
      (if (setq pos (string-match "[ ,\t\n]*\\'" userids))
        (setq userids (substring userids 0 pos)))
      ;; remove leading spaces. they bother me.
!     (if (string-match "\\s *" userids)
        (substring userids (match-end 0))
        userids)))
  
--- 247,253 ----
      (if (setq pos (string-match "[ ,\t\n]*\\'" userids))
        (setq userids (substring userids 0 pos)))
      ;; remove leading spaces. they bother me.
!     (if (string-match "\\(\\s \\|,\\)*" userids)
        (substring userids (match-end 0))
        userids)))