bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#19208: replace-match unhelpful error message


From: Lars Ingebrigtsen
Subject: bug#19208: replace-match unhelpful error message
Date: Sat, 03 Aug 2019 16:01:15 +0200
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.0.50 (gnu/linux)

phillip.lord@newcastle.ac.uk (Phillip Lord) writes:

> (progn
>   (goto-char (point-min))
>   (re-search-forward "This")
>   (replace-match "That" nil nil nil 1))
>
> This causes the following error.
>
> progn: Args out of range: -1, -1
>
> The error is caused because I have asked for a subexp that doesn't exist in
> the regexp. A programmer error for sure, but the Args out of range error seems
> rather unfriendly to me, especially as I have no idea where the -1, or
> -1 has come from.
>
> "Attempt to replace regexp subexpression that doesn't exist", for
> example, would be nicer.

Yup.

This is the code that signals the error:

      if (search_regs.start[sub] < BEGV
          || search_regs.start[sub] > search_regs.end[sub]
          || search_regs.end[sub] > ZV)
        args_out_of_range (make_fixnum (search_regs.start[sub]),
                           make_fixnum (search_regs.end[sub]));

Which is a weird way to check for this, surely?  It looks more like a
sanity checks that checks whether start[sub] and end[sub] is inside the
buffer, and that the start is before the end.

This happens to actually work here because when there is no match,
start[sub] is -1, which is always less than BEGV.

And this explains where the -1 in the args out of range comes from.

The sanity check seems sensible (because the buffer may have been
narrowed before calling replace-match), so I propose to add an
additional check that actually explicitly checks whether the subexp is
valid.

Hm...  or is this what the check right above it tried to do?

  if (NILP (subexp))
    sub = 0;
  else
    {
      CHECK_FIXNUM (subexp);
      if (! (0 <= XFIXNUM (subexp) && XFIXNUM (subexp) < search_regs.num_regs))
        args_out_of_range (subexp, make_fixnum (search_regs.num_regs));
      sub = XFIXNUM (subexp);
    }

Yoda conditionals and a !...  Let's see...

No, that's just another sanity check -- num_regs is the max allowed
number of sub-matches.  (I've now added some comments to clarify.)

I've now added a new check to check for this situation, and I've used
your suggested error string.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no





reply via email to

[Prev in Thread] Current Thread [Next in Thread]