Skip to content
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

DTD elements are not formatted #221

Closed
fbricon opened this issue Nov 13, 2018 · 13 comments
Closed

DTD elements are not formatted #221

fbricon opened this issue Nov 13, 2018 · 13 comments
Assignees
Labels
bug Something isn't working DTD formatting This issue or enhancement is related to formatting support in progress
Milestone

Comments

@fbricon
Copy link
Contributor

fbricon commented Nov 13, 2018

Following up on #90, given:

<?xml version="1.0"?>
<!DOCTYPE note [
<!ELEMENT note (to,from,heading,body)>
    <!ELEMENT to (#PCDATA)>
        <!ELEMENT from (#PCDATA)>
                <!ELEMENT heading (#PCDATA)>
            <!ELEMENT body (#PCDATA)>
]>
<note>
    <from>Jani</from>
    <heading>Reminder</heading>
    <body>Don't forget me this weekend</body>
</note>

Formatting the document ignores the DOCTYPE content.

@fbricon fbricon added bug Something isn't working formatting This issue or enhancement is related to formatting support DTD labels Nov 13, 2018
@angelozerr
Copy link
Contributor

angelozerr commented Nov 14, 2018

@NikolasKomonen to support DTD completion #106 I need to improve XMLParser to collect information of DocumentType (see https://en.wikipedia.org/wiki/Document_type_declaration)

The general syntax for a document type declaration is:

<!DOCTYPE root-element PUBLIC "FPI" ["URI"] [ 
<!-- internal subset declarations -->
]>

or

<!DOCTYPE root-element SYSTEM "URI" [ 
<!-- internal subset declarations -->
]>

We need not to work together about this parsing feature, so I tell me when you want to work on this format issue. I'm working on DTD completion, so I could start this parsing documenttype feature, but if you have started something, please tell me. Is there any chance that you could work now on this parse feature, because I need it now? If no tell me, I will start something.

@NikolasKomonen
Copy link
Contributor

@angelozerr yes, I can start working on the parsing for DTD. Ill start now.

@angelozerr
Copy link
Contributor

Thanks @NikolasKomonen!

@NikolasKomonen
Copy link
Contributor

@angelozerr Just wanted to check if you needed this as soon as possible, or is it fine if I work on another issue first?

@angelozerr
Copy link
Contributor

@angelozerr Just wanted to check if you needed this as soon as possible, or is it fine if I work on another issue first?

I can manage with a uggly mean the extract of DTD system uri if you prefere working on another issues.

@NikolasKomonen
Copy link
Contributor

@angelozerr Its fine, I'll be able to work on this now. The other issues are fixed now.

I just want to be sure, the parser should also cover this, eg: <!ELEMENT foo (#PCDATA)> for doctypes?

@angelozerr
Copy link
Contributor

I just want to be sure, the parser should also cover this, eg: for doctypes?

For my issue with XML completion based on DTD, I don't need this feature. Me I need to get public, system uri. In otherwords, implement:

My first goal is to support external DTD. I will see after how to support internal DTD (DTD declared in the XML)

@fbricon fbricon added this to the v0.0.3 milestone Nov 15, 2018
@fbricon fbricon added the to do label Nov 15, 2018
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Nov 15, 2018
Fixes part of eclipse-lemminx#221

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Nov 15, 2018
Fixes part of eclipse-lemminx#221
Doctype parsing setup. Internal DTD not complete, it instead stores as string

Signed-off-by: Nikolas Komonen <[email protected]>
@angelozerr
Copy link
Contributor

@NikolasKomonen I think you can work on this formatting issue now. Now you have 3 new structures:

  • DTDElementDecl for <!ELEMENT
  • DTDAttrList for <!ATTRIBUTES
  • DOMEntity for <!ENTITY

That you can see in the outline. Now you can use it for format. Don't hesitate to update it to improve it according your need for format.

@NikolasKomonen
Copy link
Contributor

@angelozerr Just wondering why you called it 'DOMEntity' and not 'DTDEntity' ?

@angelozerr
Copy link
Contributor

@angelozerr Just wondering why you called it 'DOMEntity' and not 'DTDEntity' ?

I have done this choice because DOMEntity implements org.w3c.dom.Entity. For the other structures ELEMENT and ATTLIST, it doesn't exists interfaces? I don't know why?

But if you prefer, we can rename it. @fbricon what do you think about that?

@fbricon
Copy link
Contributor Author

fbricon commented Nov 27, 2018

if DOMEntity is meant to be used outside the context of DTD, then you can keep it. Else, if it's specific to DTD usage, renaming it is fine

@angelozerr
Copy link
Contributor

I thinkit's for only DTD. Let's go to rename it.

@fbricon
Copy link
Contributor Author

fbricon commented Dec 1, 2018

Formatting now breaks the document:
broken-formatting

<!DOCTYPE features [
<!ELEMENT features (feature*)>
<!ELEMENT feature (#PCDATA)>
]>
<features>
	<feature>Validation</feature>
	<!-- this is 
	a comment 
	on multiple
	lines -->
	<feature>Highlighting</feature>
	<feature>Outline</feature>
					<feature>Formatting</feature>
					
	<feature>XSD support</feature>
	<feature>... and more</feature>
</features>

NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Dec 21, 2018
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Dec 21, 2018
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 2, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 3, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 3, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 3, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 3, 2019
@fbricon fbricon added in progress and removed to do labels Jan 7, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 7, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 7, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 8, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 8, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 8, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 9, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 9, 2019
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 9, 2019
fbricon pushed a commit that referenced this issue Jan 9, 2019
Fixes #221 and #268

Signed-off-by: Nikolas Komonen <[email protected]>
NikolasKomonen added a commit to NikolasKomonen/lsp4xml that referenced this issue Jan 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working DTD formatting This issue or enhancement is related to formatting support in progress
Projects
None yet
Development

No branches or pull requests

3 participants