Subject: RE: bug#24914: 24.5; isearch-regexp: wrong error message

From: Drew Adams <[email-masked]>
Date: Mon, 4 Dec 2017 19:15:42 -0800 (PST)

> > What I cited are cases where we do flag _particular kinds_
> > of invalid regexps, and so tailor the error msg.
> 
> I'm not sure if you're citing actual code we have right now, or just
> some hypotheticals.  In isearch.el, we pretty much do the opposite of
> tailor the error message.

I was citing what I thought were such cases in the current
isearch.el code - cases where we do not just say "Invalid
Regexp".  We say things like this:

 Too many words
 Too many spaces for whitespace matching
 Unmatched [ or [^

Granted, the last is used only in `isearch-query-replace.

My point was that in some existing cases (not many,
admittedly), we do try to give a more precise error
message when signal `invalid-regexp' is detected.

But I'm not sure what you're arguing, if you are arguing.

Certainly we don't tailor the message _much_ for the
kind of `invalid-regexp'.  But we do make some effort
to do that, even now, AFAICT.

> >> Actually, I'm wondering what's the point of isearch showing
> >> "incomplete" instead of the actual regexp invalid error.
> >> I.e., why not instead of
> >>
> >>     \ [incomplete]
> >>     \{ [incomplete]
> >>     \{4 [incomplete]
> >>     \{4000 [incomplete]
> >>     \{4000\ [incomplete]
> >>     \{4000\}
> >>
> >> show this:
> >>
> >>     \ [Trailing backslash]
> >>     \{ [Unmatched \{]
> >>     \{4 [Unmatched \{]
> >>     \{4000 [Unmatched \{]
> >>     \{4000\ [Trailing backslash]
> >>     \{4000\}
> 
> I meant just the following patch, you can try it out easily:
>      (invalid-regexp
>       (setq isearch-error (car (cdr lossage)))
>       (cond
> -      ((string-match
> -	"\\`Premature \\|\\`Unmatched \\|\\`Invalid "
> -	isearch-error)
> -       (setq isearch-error "incomplete input"))
>        ((and (not isearch-regexp)
>  	    (string-match "\\`Regular expression too big" isearch-error))
>         (cond

You mean show "[Invalid content of \{\}]" in all cases?
_Never_ show "[incomplete input]"?  Why would that be better?

Anyway, I don't have a strong opinion about that.  I do think
that in the case reported it's too bad that we say "[incomplete
input]".

But I don't think it follows that it would be more helpful to
most users to show the invalid-regexp description in cases
where Emacs can really tell that the input is necessarily
incomplete.  I suspect that it is quite common for that
"incomplete input" message to be helpful.