Formatting breaks attributes containing quotes

[tkovacs-dev]

How to reproduce:

1. Use this document: <hello attr='"' />
2. Run the "Format document" command, with xml.format.quotations set to doubleQuotes (the default)

Symptoms:
The formatting produces the following invalid XML: <hello attr=""" />

Expected:
Formatting shouldn't change the type of quote used for an attribute if that produces invalid XML, that is if the attribute value contains the quote character.

[angelozerr]

I agree with you, but I think it's not a commons usecase.

[mcn-mark-gordon]

It also affects me. Due to the nature of the system I am developing stylesheets for I sometimes need to use double-quotes in attributes and sometimes need to use apostrophes in them.

It might not be common (I don't know), but it is certainly more than one person affected.

[brianary]

Whether or not it was a good design decision, MuleSoft XML config files embed their DataWeave scripting language (which uses a lot of quotes) in attributes of their code. This indicates that an entire community is affected by this bug.

[fbricon]

@brianary can you please provide an example of such config file?

[brianary]

Here is one example element from a recent project:

<set-payload
		value='#[output application/json
---
{
	"message": "Error while creating the record",
	"Message Text": error.muleMessage.typedValue default "Error Occured"
}]'
		doc:name="Set Payload"
		doc:id=" 6d5b1304-f21c-4e3f-83af-655d8b5293c6 " />

DataWeave is primarily a tool for mapping one object structure to another:

<set-payload
		value='#[output application/json
---
{
	"Message": "Record created successfully",
	"ID": payload..id[0],
	"Type":payload..type_name[0],
	"Message Severity":payload..MsgSeverity default "No Error",
	"Message Text": payload..MsgText joinBy ";" default "Success"
}]'
		doc:name="Set Payload"
		doc:id=" 010e9893-7c8c-416d-bfa9-f4c48df6b6b4 " />

[angelozerr]

Thanks @brianary for your sample. But even if we fix this issue, I think the vscode-xml formatting will not be enough, because I suppose you want to format the attribute value which can contain JSON content.

And I suppose you would like to have syntax coloration for this JSON content, no?

Those 2 issues (which are out of the scope of this issue) requires to provide some extension (particpant for format feature and TextMate injection for the synax coloration) which will not very simple to fix.

@fbricon @xorye perhaps we should fix this issue soon since more and more people complain with this quote problem?

[brianary]

What? No. It is DataWeave, not JSON (though it is similar), but in neither case do I need syntax and highlighting support. Often (not primarily) that code is manipulated via a GUI, but that formats the XML in ways that don't diff well within git/GitHub. I just want to be able to use this extension (the only one that does XML folding right) to format the XML more predictably/consistently without it breaking our code. This is not a slippery slope.

…

On Wed, Apr 15, 2020, 12:29 Angelo ***@***.***> wrote: Thanks @brianary <https://github.com/brianary> for your sample. But even if we fix this issue, I think the vscode-xml formatting will not be enough, because I suppose you want to format the attribute value which can contain JSON content. And I suppose you would like to have syntax coloration for this JSON content, no? Those 2 issues (which are out of the scope of this issue) requires to provide some extension (particpant for format feature and TextMate injection for the synax coloration) which will not very simple to fix. @fbricon <https://github.com/fbricon> @xorye <https://github.com/xorye> perhaps we should fix this issue soon since more and more people complain with this quote problem? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#182 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB7SMJYCLSOC3J5C72FXRCTRMYDI3ANCNFSM4IYKPB2A> .

[angelozerr]

I just want to be able to use this extension
(the only one that does XML folding right) to format the XML more
predictably/consistently without it breaking our code.

nice :)

Ok now I understand more it's a blocking issue for you.

[brianary]

Thanks! I agree the choice of putting code in an attribute is weird enough that it doesn't make much sense to try and add full language support.

…

On Wed, Apr 15, 2020, 12:41 Angelo ***@***.***> wrote: I just want to be able to use this extension (the only one that does XML folding right) to format the XML more predictably/consistently without it breaking our code. nice :) Ok now I understand more it's a blocking issue for you. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#182 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AB7SMJZH2OYCBGEROWANH6DRMYEVNANCNFSM4IYKPB2A> .

[angelozerr]

To fix this issue, we should had a new enum none for xml.format.quotations which doesn't change the quote.

[fbricon]

I'd rather prefer xml.format.quotations: existing

[angelozerr]

I'd rather prefer xml.format.quotations: existing

I suggested none to be consistent with xml.format.emptyElements : none which means do nothing.

[fbricon]

then xml.format.emptyElements should use existing too ;-)

[angelozerr]

Ok!

[fbricon]

actually, I wonder if ignore would be a better fit for all

[angelozerr]

actually, I wonder if ignore would be a better fit for all

I agree with that, I will do the changes

[angelozerr]

@fbricon after discussion with @xorye, I think xml.format.quotation should not exist. Today this settings is used

• for apply completion to generate attribute
• for formatting

The most important feature is for completion apply, so I think we should have xml.preferredQuotation setting (instead of xml.format.quotation).

After that formatting will keep the existing quotation. Now if we want to force the quotation while formatting, I think we should introduce a new setting like xml.format.usePreferedQuotation : true which will replace the quotation, but if we do that, IMHO I think we should take care of the used quote in the attribute value (perhaps not replace the quote).

[fbricon]

So, there's a javascript/typescript preference we could copy:
"typescript.preferences.quoteStyle": "auto/single/double",
doc says:

Preferred quote style to use for quick fixes: single quotes, double quotes, or auto infer quote type from existing imports. Requires using TypeScript 2.9 or newer in the workspace.

We could start by having "xml.preferences.quoteStyle":"single/double" and maybe add a way to automatically infer quote style in the future

From there, we could have:
"xml.format.enforceQuoteStyle": true/false or maybe an enum value, that would allow us to provide more behaviors in the future

[fbricon]

@Keccs you can now set "xml.format.enforceQuoteStyle"="ignore"