[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] xml-escape-region
From: |
Stefan Monnier |
Subject: |
Re: [PATCH] xml-escape-region |
Date: |
Thu, 08 Oct 2009 01:29:33 -0400 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux) |
>>> +;;;##autoload
>>> +(defun xml-escape-region (beg end)
>>> + (interactive "*r")
>>> + (let ((escaped (xml-escape-string (buffer-substring beg end))))
>>> + (delete-region beg end)
>>> + (insert escaped)))
>>
>> I'd rather not autoload such a function.
> Do you mean that it should be loaded all the time, or that the user should
> have to explicitly load xml.el before using the function?
Yes.
> If the latter, then that would make binding it to a key
> less convenient.
Hmm... didn't notice you defined it as a command. How often/when do you
need to use/bind such a command other than in an sgml/xml-related file
(where the major mode might decide to preload such a command)?
>> But more importantly, this implementation is very inefficient.
>> xml-escape-string itself is rather inefficient except for short
>> strings; this is OK for its current uses, but for xml-escape-region
>> it's definitely not good (i.e. only usable for small regions).
> How's this? It's O(N) in the amount of text escaped.
Much better, thank you.
> (let ((search-re (mapconcat #'regexp-quote
> (mapcar #'cdr xml-entity-alist)
> "\\|"))
Rather than a big \| of single chars, why not make a [...] regexp?
If you use regexp-opt, it should happen automatically.
Actually, now that I look at it, xml-entity-alist is poorly defined.
Instead of being a list of pairs of string and string (where the second
string is always of size 1), it should be a list of pairs of string
and char. Also this code is also applicable to sgml and there's related
code in sgml-mode.el. If someone wants to consolidate, that would
be welcome.
> (save-excursion
> (goto-char beg)
> (while (re-search-forward search-re end t)
> (replace-match (concat "&"
> (car (rassoc (match-string 0)
> xml-entity-alist))
> ";"))))))
If you use a backward-search, you don't need to turn `end' (nor `start')
into a marker.
Stefan