Re: Patch to emacs-w3m

[Paul Kinnucan <paulk@xxxxxxxxxxxxx>]

Jose A. Ortega Ruiz writes:
 > 
 > hi,
 > 
 > i have found the cause of the bug today, before reading your mails
 > (but thank you very much to paul and katsumi, of course), and have
 > 'fixed' it with this redefinition:
 > 
 > (defun w3m-decode-entities-string (str)
 >   "Decode entities in the string STR. -- redefinition"
 >   (save-match-data
 >     (with-temp-buffer
 >       (insert str)
 >       (w3m-decode-entities)
 >       (buffer-string))))
 > 
 > i.e., if just moved the 'save-match-data' form to the body's top. so
 > far it's working like a charm here, and i got:
 > 
 > (w3m-decode-entities-string "&amp;&lt;&gt;") => "&<>"
 > 
 > which is, i think, the expected result. although i agree with katsumi
 > in that the cause of the bug should be fixed at its origin, the above
 > workaround may make some users (as myself) happy in the meantime :)
 > 

Hello Jose,

As I stated in my original message, I already have fixed the so-called
"bug" in the JDEE sources. I put the "bug" in quotes because there is
no bug in the JDEE code. The original JDEE code works just fine
without the change that I made. It is emacs-w3m that does not work and
hence has a bug. The problem is that the original w3m-emacs code makes
the assumption that creating a temp buffer does not change existing match
data. This is a false assumption (and hence a bug) because of the
possibility of a kill buffer hook function changing the match data
after w3m-decode-entities-string finishes decoding but before it
returns to its caller.

Your fix solves that problem. However, it does not address another
problem with the function, namely the inefficiency of creating a
buffer just to decode a string. Unfortunately, my fix, while avoiding
unnecessary buffer creation, was not general enough as it did not
handle the cases where a string contains multiple entities or entities
embedded in other text. I intend to contribute a version of
w3m-decode-entities-string that handles the general case without
creating a temporary buffer. The maintainers of emacs-w3m should feel
free to ignore my contribution. However, I certainly hope that they
will accept either your fix or my rewrite of
w3m-decode-entities-string to avoid this needless inter-package
compatibility problem in the future.


Paul

 > cheers,
 > jao
 > 
 > Katsumi Yamaoka <yamaoka@jpl.org> writes:
 > 
 > >>>>>> In [emacs-w3m : No.05911] Katsumi Yamaoka wrote:
 > >
 > >> Thanks for the patch to the w3m-decode-entities-string function.
 > >> I've installed it in the emacs-w3m CVS repository.
 > >
 > > I'm sorry to inform you that I've canceled the last change to
 > > w3m.el.  TSUCHIYA Masatoshi, the chief maintainer said that the
 > > change causes the following fault:
 > >
 > > (w3m-decode-entities-string "&amp;&lt;&gt;")
 > >  => "&"
 > >
 > > It should be:
 > >
 > > (with-temp-buffer
 > >   (insert "&amp;&lt;&gt;")
 > >   (w3m-decode-entities)
 > >   (buffer-string))
 > >  => "&<>"
 > >
 > > I think the cause by which the with-temp-buffer macro doesn't
 > > work properly should be solved by the origin rather than
 > > emacs-w3m.  So, I withdrew the last change in CVS.  Please don't
 > > think badly of me.
 > > -- 
 > > Katsumi Yamaoka <yamaoka@jpl.org>
 > >
 > 
 > -- 
 > "Knowledge is proud that he has learned so much;
 > Wisdom is humble that he knows no more." -- William Cowper