Re: error in replace-match: "args out of range"

[Tim X <timx@nospam.dev.null>]

ken <gebser@mousecar.com> writes:

> On 04/04/2011 06:21 PM Tim X wrote:
>> ken <gebser@mousecar.com> writes:
>> 
>>> On 04/03/2011 07:19 PM PJ Weisberg wrote:
>>>> On 4/3/11, ken <gebser@mousecar.com> wrote:
>>>>> On 03/28/2011 04:32 PM Stefan Monnier wrote:
>>>>>> I'm just saying that
>>>>>>
>>>>>> M-: (some-regexp-search) RET
>>>>>> M-: (replace-match ...) RET
>>>>>>
>>>>>> won't always do what you want, because a whole lot of code will run
>>>>>> between the two.
>>>>>>
>>>>>>
>>>>>>         Stefan
>>>>> 
>>>....
>>>
>>> So, yes, the search and replace-match are in the same function.  Are
>>> they "close together"?  I'd say yes, but that's a judgment call really.
>>>  The string I want to replace in this particular instance is a null string.
>>>
>> 
>> Providing the code alway helps. While Stephan's points are valid and a
>> common error people make, I don't think it is the problem you are
>> seeing.
>
> Yes, providing the actual code is generally a good idea... it gives
> everyone something concrete to look at.  But the function I was writing
> was too long to expect anyone to read and digest it in just a few
> minutes; it would also require a lot of explanation from me to help
> others to understand it; three or four ancillary functions play roles
> and so they would have to be included also; all of this required a
> specific kind of text file to work it on, so I would have need to
> include that also; and besides all this, the code was a very unappealing
> mess to look at, with all kinds of cruft commented out, sundry comments
> inserted here and there, and some code not formatted in the traditional
> fashion.  In short, it would have been too much work for everyone.  So I
> thought just posting the directly relevant code fragments would be more
> effective.  When there's a working package, a version 0.0001, I'll post
> it on the web so anyone who's curious or wants to use it can do so.
>

An approach that can work is to boil down the problem to the smallest
bit of code that will reproduce the error and post that. To some extent,
I guess that is what you did. When asking for help with code related
problems, it really is the only way to go. 


>
>> 
>> It would help to see the regexp you are using to test this. Note that
>> match-string returns nil if the subexpression you are asking for did not
>> exist. If the subexpression did not exist, what would replace-match
>> replace as there is no match?
>
> Tim, this is a valid point generally, but not in this instance.  The
> code included conditionals for each search and so if the search failed
> to find the sought RE, a different block of code would be executed.  And
> there was a block of code to cover each of the five possible conditions.
>

The reason I asked for a copy of the regexp is that regexps can be
tricky and it really helps to also have, in addtion to the code, the
regexp being used as input when the code generates the error. 

For example, you can have a regexp with one or more  subexpression that does not
match while your overall regexp did match. According to the docs, in
that case, match-string would return nil when queried for the
subexpressions and not "". Note that "" and nil are not
the same. Depending on your regexp, both aname-str and head-text-nested
could have the value nil. However, your conditional statements that use
these variables only check for "", which is not the same as nil and
therefore could be returning true/false unexpectedly and causing the
wrong branch to be executed. 

This may or may not be an issue, depending on your regexp. 

>
>> 
>> Note also, you can get rid of some of those progns i.e.
>> 
>>      (when (re-search-forward aname-re-str head-text-end-pt t)
>>  	  (setq aname-str (match-string 2))
>>  	  (setq head-text-nested (match-string 4))
>>  	  (when (string= head-text-nested "")
>>  		(setq head-text-nested (create-heading-text))
>>                 (if (not (string= aname-str ""))
>>  		     (replace-match head-text-nested t t nil 4)
>>                   (setq aname-str (create-aname head-text-nested))
>>  		  (replace-match head-text-nested t nil nil 4)
>>  		  (replace-match aname-str t nil nil 2))))
>> 
>> Note that the 'else' part of an if form has an implicit progn, so by
>> revesing the condition test, we can eliminate that progn. The 'unless'
>> form can be used instead of 'when' if you want to reverse the logic of
>> what you are doing. 
>
> Yeah, inverting the conditionals does remove the progn statements and
> makes the code shorter.  I read quite a bit about conditionals in some
> of the docs... and someone here on this list explained about progn a bit
> for me also.  But I wrote the code how I was thinking through the
> problem, much in the way Kernighan and Ritchie say happens naturally...
> I think it was in the "Introduction" to their book, "The C Programming
> Language".  That "progn" statement does seem clunky... it took some
> getting used to using an entire word just to denote the beginning of a
> block of text.  But I've adapted.  In fact, now it's like a huge flag
> telling me, 'this is where the block of text begins which executes when
> the if statement above is true.'  It makes it easier to find those if's
>  too.  Sometimes I almost wish there was a corresponding "<progn" to
> just as boldly show me where the end of the code block is.  But it's too
> late to design the syntax for lisp :) and I'd probably get tired of
> "<progn" after a time anyway.

Another way to look at it is if you think you need progn, then maybe you
actually need a different conditional. This often makes your code even
more readable. For example (when something do-this-block) or (unless
something do-this-block) rather than (if something progn do-this-block)
or (if not something progn do this block). 

progn does have a role to play, but less than many realise. When first
learning lisp, it is one of those 'flags' you can use to alert you there
are better ways to express what you are trying to say. It also has the
advantage of less indentation and lines of code on the screen, making it
easier to 'see' the code in one glance. For list, this is quite
important as it is the ability to see the code and in particular, the
indentation to get a sense of the code.

>
>
>> 
>> (Note above is untested, but intention should be clear)
>> 
>> My guess is that it does not work because replace-match has no idea what
>> it is yhou want to match as the subexpression was not found i.e. is not
>> in the string, so it has no way to know where in the string to do the
>> replace.
>
> When the subexpression isn't found, then the search fails.  When the
> search fails, then a different code block executes, one which doesn't
> require the existence of that subexpression.
>

This was my point of warning. It is quite possible to have
regexps where sub expressions are not found, but the overall regexp
succeeds. This means that the regexp-search-* will succeed, but the
functions to extract the subexpressions will return nil. As we dind't
have an example of the regexp you are using, it is not possible to know
in this case if this is an issue or not. If your code will allow the
user to define the regexp, you would need to test for nil and not just "".

> In addition, I wrote a simple diagnostic function (a couple versions of
> it actually) to show me what, if anything, was matched and inserted this
> function at various points in the code.  This confirmed that the
> searches and conditionals were functioning as I wanted.  Something else
> was wiping out the subexpressions.  I eventually traced the problem to
> the read-from-minibuffer function.  For some reason it wipes out
> previously established subexpressions.  I wasn't expecting that at all.
>

I guess that was Stefan's point. You can not know what other functions
do 'under the hood' and therefore don't know when regexp searches are
performed and when the match data may be overridden. Therefore, you must
always assume the data is volatile and either use it immediately or
store the relevant details in local variables that you control or use
something like save-match-data etc

> Tim, thanks for all the input... and everyone else too.  Don't worry...
> there's quite a bit more code I need to write, so I'll most likely be
> posting more questions/problems in the upcoming.  (Is that something
> anybody's worried about? :)  )
>

Writing code is the only way to learn and in general, most people seem
happy to help anyone who is showing effort to learn and understand.
Besides, this is usernet, anyone who isn't interested can just skip
those messages.

Tim


-- 
tcross (at) rapttech dot com dot au