[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Patch to emacs-w3m
Jose A. Ortega Ruiz writes:
> 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"
> (insert str)
> 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 "&<>") => "&<>"
> 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 :)
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.
> Katsumi Yamaoka <firstname.lastname@example.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 "&<>")
> > => "&"
> > It should be:
> > (with-temp-buffer
> > (insert "&<>")
> > (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 <email@example.com>
> "Knowledge is proud that he has learned so much;
> Wisdom is humble that he knows no more." -- William Cowper