emacs-devel
[Top][All Lists]
Advanced

[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: Fri, 09 Oct 2009 15:11:16 -0400
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1.50 (gnu/linux)

> Pretty often, actually. XML (or XML-like syntax) crops up in a lot of
> places, including
> literal strings in many programming languages. Some basic XML-editing
> functionality being available everywhere would be useful.

OK, then.

>>> (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.
> I figured the constant-factor overhead of regexp-opt (and its autoloading)
> wasn't worth it for such a simple regexp.

Not sure if it makes a difference, indeed.

>> 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.
> I think the idea was to be able to replace multi-character strings with XML
> entities defined for the current document.

All the examples I saw were single-char strings.  If indeed multi-char
strings can be used there, then the current definition is fine, indeed,
but then xml-escape-string is not.

> Does anyone actually use the unquotep parameter? It seems like quoting and
> unquoting should be separate functions.

Separate functions yes, but I think it makes sense to provide both under
the same key-binding, hence a single command.  This said, I never use
either, so I don't have a strong opinion.

> Nevertheless, the patch below should preserve existing behavior. I've
> also renamed the XML functions to better match existing code,
> e.g., base64.

Looks good,


        Stefan




reply via email to

[Prev in Thread] Current Thread [Next in Thread]