Bug #9367
closedREXML::XmlDecl doesn't use user specified quotes
Description
=begin
REXML uses double quotes for quoting attributes if :attribute_quote is specified as document's context like below:
doc = REXML::Document.new
doc.context[:attribute_quote] = :quote
This looks working well on all elements but has no effect for xml declaration (i.e. ) if it exists.
Even if I specify (({doc.context[:attribute_quote] = :quote})), I will get this:
The expected result is:
=end
Updated by duerst (Martin Dürst) over 10 years ago
- Status changed from Open to Feedback
Strictly speaking, the XML declaration doesn't contain any attributes, only things that look like attributes. They are sometimes called pseudo-attributes. So it is not unreasonable that
doc.context[:attribute_quote] = :quote
does not affect pseudo-attribute quoting.
For attributes, the choice of quoting can affect the amount of escaping. But this doesn't apply to pseudo-attributes, as the 'values' in the pseudo-attributes don't contain quotes.
Can you explain the specific reason you want double quotes in the XML declaration?
Updated by bearmini (Takashi Oguma) over 10 years ago
A direct reason is that my customer wants to have double quotes in the xml declaration because their handcrafted 'xml lint' tool complains the xml document produced by my ruby script contains single quotes. (Their convention requires all quoting characters should be double quotes.)
More generally, I think it is natural if we have control which quoting character will be used for the xml declaration too.
Updated by duerst (Martin Dürst) over 10 years ago
bearmini (Takashi Oguma) wrote:
A direct reason is that my customer wants to have double quotes in the xml declaration because their handcrafted 'xml lint' tool complains the xml document produced by my ruby script contains single quotes. (Their convention requires all quoting characters should be double quotes.)
That's a very good reason, in particular for you.
More generally, I think it is natural if we have control which quoting character will be used for the xml declaration too.
It probably won't hurt if this is controllable. But there might be some existing applications (and tests) that expect single-quoted pseudo-attributes in XML declarations, and they would get problems if
doc.context[:attribute_quote] = :quote
changes that. So I think it would be better if it's something like
doc.context[:xml_declaration_quote] = :quote
Anyway, I don't have the time to prepare a patch, sorry. But maybe you can create a patch?
Updated by kou (Kouhei Sutou) over 10 years ago
- Status changed from Feedback to Assigned
- Assignee set to kou (Kouhei Sutou)
duerst (Martin Dürst) wrote:
It probably won't hurt if this is controllable. But there might be some existing applications (and tests) that expect single-quoted pseudo-attributes in XML declarations, and they would get problems if
doc.context[:attribute_quote] = :quote
changes that. So I think it would be better if it's something like
doc.context[:xml_declaration_quote] = :quote
I also prefer to :xml_declaration_quote.
Anyway, I don't have the time to prepare a patch, sorry. But maybe you can create a patch?
I comment his patch: https://github.com/ruby/ruby/pull/496
I'm happy that if he applies my comments but it is OK that he doesn't care about them. I will apply my comments after I merge his patch. :-)
- You can use "@parent" instead of "parent" because "@parent" is widely used in REXML internal.
- I don't like one character variable. :<
- "and" is REXML style rather than "&&". (But "||" is used in XMLDecl#content...)
- I prefer to one assertion (one check item) in one test rather than multiple assertions in one test.
- It seems that you don't need to use "xml" created in "setup".
Updated by bearmini (Takashi Oguma) over 10 years ago
@duerst (Martin Dürst), @kou (Kouhei Sutou),
Thanks for the comments!
I'll revise my patch to use :xml_declaration_quote and back to you soon.
Updated by bearmini (Takashi Oguma) about 10 years ago
Hi,
I'm working on this, and ran into another issue. i.e. should we be able to control single quote or double quote for DOCTYPE? If so, how?
According to http://www.w3.org/TR/xml/#NT-doctypedecl, DOCTYPE may contain ExternalID which may contain SystemLiteral or PubidLiteral, they can be quoted either single quote or double quote.
Should we introduce another symbol such as :xml_doctype_quote, or change the :xml_declatation_quote to :xml_prologue_quote or something?
Updated by duerst (Martin Dürst) about 10 years ago
Takashi Oguma wrote:
Should we introduce another symbol such as :xml_doctype_quote, or change the :xml_declatation_quote to :xml_prologue_quote or something?
Either way is fine by me, but maybe it's best to just use one symbol (:xml_prologue_quote) until we find a use case for separating.
Updated by bearmini (Takashi Oguma) about 10 years ago
Hi,
I have updated my PR https://github.com/ruby/ruby/pull/496
Please review it and comment on that.
Updated by kou (Kouhei Sutou) about 10 years ago
Thanks!
But I can't merge the pull request because it changes public API, NotationDecl.new
. I don't want to add backward incompatible API change.
I'll comment furthere on the pull request later. Sorry.
Anyway, thanks for your work!
Updated by kou (Kouhei Sutou) about 10 years ago
-
include StringQuotes
afterprivate
is meaningless. Please put it the top of class definition. - It seems that
strip_quotes
is needless foradjust_prologue_quotes
. Can we just remove.inspect
fromadjust_prologue_quotes XXX.inspect
? If we can do it,adjust_prologue_quotes
isn't good name.quote
or something will be good name. - Please don't omit parenthesis for method call in
#{...}
. - Please don't set
DocDecl
asNotationDecl
's parent. (It was mentioned at #9367-9.) How about callingcontext
method instead of getting context from@parent
inadjust_prologue_quotes
? It seems thatadjust_prologue_quotes
is too depends on internal implementation.
(Sorry for my many comments...)
Updated by duerst (Martin Dürst) almost 9 years ago
- Has duplicate Bug #11176: Hardcoded settings in REXML added
Updated by kou (Kouhei Sutou) over 5 years ago
- Status changed from Assigned to Closed
- Backport deleted (
1.9.3: UNKNOWN, 2.0.0: UNKNOWN, 2.1: UNKNOWN)
I've implemented :prologue_quote
context option. Available values are :quote
and :apostrophe
.
https://github.com/ruby/rexml/commit/e89702294881a8aedb15905d8a85137876749f7f