-
-
Notifications
You must be signed in to change notification settings - Fork 33
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
0.4.0: indentation issues #40
Comments
Okay so a couple of things are going on here. Technically, the plugin is functioning as designed. Though I realize it's not the best user experience at the moment. Unfortunately in XML whitespace is very significant unless you're inside of With lines 21-23, 25, 29, and 30, you're actually seeing where the indentation would be if whitespace were insignificant. I'm really not entirely sure of the way around this. I've been thinking maybe an |
Thanks for the explanation! To dumb it down for me (I am not the XML expert as you probably already noticed...): Row 8 in the result is indented by two spaces because row 7 in the original is also indented by two spaces? Meaning In that case I agree with your idea of having an option for that, so that PS: preserving the indentation inside multi-row strings is of course correct, I am not talking about those! |
So basically, when you have something like:
in XML that means that the content inside of the
Now the content inside of the |
So yeah, basically I'm thinking of trying to provide a rule that says if you don't have mixed content inside the tags, then we could probably just consider the whitespace as insignificant and indent as we like, but right now it would definitely change things otherwise and potentially break XML integrations. |
Specification wise XML is indeed whitespace sensitive. However what % of real XML files are really whitespace sensitive? If we do not ignore whitespace it is very difficult to get a real "pretty XML"... This issue is what stumped me when I was playing around with a prettier plugin for |
I think it pretty much has to be an option. Otherwise this is really only prettier-ifying open/close tags and prolog content. |
Now to figure out what to name it. |
For HTML, the analogous option of Prettier is called Also schema languages (DTD, XML Schema, RELAX NG, Schematron) are used to define, among other things, whitespace rules for elements. If the document has an associated schema, you should be able to get the needed info about whitespace significance from there. Finally, you can probably somehow use the standard |
My 2 cents: I would go with the option name
And use "ignore" by default and see if any users complain. 😄 I'd probably also emphasize this option and its default behavior in the documentation. |
Sounds good for starters. The "css" value would make sense too as XML is one of the two syntaxes of HTML5. And it might be possible to come up with a simple, but good enough heuristic for another value, "guess". E.g. "if an element doesn't have any non-whitespace text nodes among its children, then its whitespace-only child text nodes can be ignored". Something like that. |
That sounds very reasonable. In our specific case, all whitespaces are insignificant, except for those inside multi-row strings. So if a value of |
@winterkind BTW, did you try formatting your files with Prettier's HTML formatter and |
@thorn0 You are right - that looks very good, also with our larger files. It just needs some convincing at this point. The "babel" parser complains about HTML comments and the explicit HTML parser has issues with our default namespace not being HTML. But as I said: with the "babel" parser and after removing comments, the output is excellent. |
@winterkind I meant the |
@thorn0 I don't think there is anything wrong with the HTML parser. To quickly show what I meant see here. It is XML with a given default namespace and a I liked your comment because it showed the effect that a whitespace option could have for our XML. |
I am unsure if HTML is always a valid (syntactically) XML. |
@bd82 It's usually not, but that doesn't mean Prettier's HTML parser (Angular's actually) can't be smart enough to support XML. It's something I'd like to investigate when I get some time. |
Re. #40 (comment):
Yep, proper context-sensitive whitespace handling is basic table stakes for an XML processing toolchain, so a simple on/off, strict/ignore approach wouldn't be sufficient, the plug-in would need to toggle the mode based on the current element.
The schema languages typically handle that for each XML dialect, so the schema will typically use For example, from the DITA spec:
|
Okay, I think what I'd like to go with is creating an |
@winterkind would you be able to test the |
Actually I've just gone ahead and released a |
Hi, sorry for not responding earlier; I was sick for a few days. I really appreciate your help with this. I tried 0.6.0 with the new option and it looks very good. I'll test with some more of our files and if I find anything else, I'll create new issues. |
Using 0.4.0 I tested one of our XML files.
Settings:
Before:
After:
Perceived issues:
tabWidth
of 2 seems to be used.The text was updated successfully, but these errors were encountered: