[mythtv] [mythtv-commits] mythtv commit: r16256 by nigel

[Michael T. Dean]

On 02/26/2008 10:05 PM, Nigel Pearson wrote:
>> Heh.  Didn't see the change in [15982], but technically, this wasn't a
>> typo.  The code checks for "<p>" tags or the "<br" part of a tag to
>> catch either "<br>" or "<br />".
>>     
> Tricky. Of course, I didn't read the code :-)
> I was just auditing the diffs between trunk & fixes.
>
> ...
>   

Yeah.  It definitely looked like a typo when just reading the README.

Oh, and I wasn't blaming you (or anyone else for that matter).  Though
your name was on the message, it was only because I noticed the change
in your commit (because that's all that was in your commit).  In truth,
though the README was technically accurate, it wasn't well explained, so
it /was/ wrong.

>> That being said, I don't think it's worth changing the README back  
>> as it
>> probably makes more sense to the user in its current "corrected" form,
>> even if it's not as precise as before.  As long as we don't  
>> "correct" it
>> in httpstatus.cpp, it should be fine.  :)
> I will try to remember that!
>
>
> Probably worth a comment? e.g.:
>
>               if ((display.contains("<p>", false) > 0) ||
> -                (display.contains("<br", false) > 0))
> ===
>               if ((display.contains("<p>", false) > 0) ||
> +                (display.contains("<br", false) > 0))   // matches  
> <br> or <br />
>   

Sounds good to me.  I can do up a patch if you like (though for that, it
may be more work for you to apply the patch than to copy/paste it in).

And, since I've publicly admitted a couple of shortcuts/assumptions I
made, I'm willing to do a patch with the comment that also fixes them,
if necessary.  So, if anyone has any concerns about not checking for
"<P>" or "<BR" (I think we can ignore "<Br" or "<bR") I could put checks
for them in, too.  Oh, and I'm relatively sure there are no other HTML
tags that start with "<br" that could mess up that check, but if someone
knows of one (i.e. something new in HTML 5)...

> In fact, the whole file could do with some Doxygenation.
> Maybe in my spare time (when I add support for <P> and <BR :-)

Yeah.  I followed the "local" style when modifying that file.  :)  At
least I did try to do some doxygen comments in the dbutil stuff I
created from scratch (though I haven't actually run it through doxygen...).

Mike