What's wrong with this subroutine?

[Joshua Macy]

A.Newby wrote:

> First, here's what I'm trying to do ......
> 
> I'm working on a html, cgi based chat script. It's working pretty well so 
> far. 
> 
> I want to get this subroutine so that the user just has to post a url into 
> the chat box to get a post a picture or a link in chat. For e.g, to post a 
> pic of a walrus, you would simply type ....
> 
> http://www.pbs.org/kratts/world/oceans/walrus/images/walrus.jpg
> 
> ... and the script would take that, and before writing it to the chat log 
> it would convert it to ...
> 
> <img src = http://www.pbs.org/kratts/world/oceans/walrus/images/walrus.jpg>
> 
> ... or to post a link, the user would simply type ... 
> 
> http://www.goodvibes.com/
> 
> ... and the script would convert it to ....
> 
> <a href = "http://www.goodvibes.com/>Link!</a>
> 
> 
> Here's the subroutine I'm using to try and do this.... it is called from a 
> function named "infilter" which filters the posts before writing them to 
> the chat log ...
> 
> def search4URL(string):
>     #the following bits are used to alter the post when it finds the URL
>     beginpic = " <img src =" 
>     endpic = ">"
>     beginlink = " <a href ="
>     endlink = " target = blank>Link</a>"
>     
>     URLStarts = string.find(' http:') #space is so when the function     
> 	    	    	    	    	    	  #repeats, it won't convert the same 
>     	    	    	    	    	    	  #block of text again
> 



   What happens when there are no more ' http:' patterns in your string 
and string.find returns -1?  I would try this either with a regular 
expression (r'(http://\S+)' would probably do the trick), or in a while 
loop that makes sure that there is something to replace before trying to 
replace it:

while string.find(filterstring, ' http:') > -1:
     filterstring = search4URL(filterstring)

or even by chewing up the string so the search doesn't have to keep 
starting from the beginning:

newstring = ''
while filterstring:
    nextHttp = string.find(filterstring, ' http:')
    if nextHttp > -1:
         newstring = newstring + filterstring[:nextHttp]
         linkstring, filterstring = search4URL(filterstring[nextHttp:]) 

         newstring  = newstring + linkstring
    else:
       newstring = newstring + filterstring

where search4URL returns the link string that it constructs and the 
remainder of the filterstring seperately.

   BTW, just as a stylistic matter, it's probably better not to shadow 
the string module with a local variable named 'string' inside your 
function.  This works in your current function because 'string' contains 
a string, and strings now all have methods such as find that have the 
same name as the module-level function (i.e. 'this is a 
string'.find('this') does the same thing as string.find('this is a 
string', 'this')), but that's not generally the case--if you 
accidentally did this with a module like math or re it would be a bug. 
Also, the intent would be clearer if you called the function something 
like replaceURLwithLink instead of search4URL, since it doesn't just 
search for the url.

   I hope this helps,

   Joshua