-
Notifications
You must be signed in to change notification settings - Fork 93
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 parsing/scanner updated. More properties for DTD types implemented. #261
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please fix the commit message
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMDocumentType.java
Show resolved
Hide resolved
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DOMParser.java
Show resolved
Hide resolved
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DTDNode.java
Outdated
Show resolved
Hide resolved
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/TokenType.java
Outdated
Show resolved
Hide resolved
@NikolasKomonen I have not tried your PR, but the most important thing is to support tolerant parser otherwise outline will not display the other nodes after the DTD content malformed. In other words, please check at first that a malformed DTD content (ex : <!ATTRLIST name) continue to parse the next content in the outline. For instance
must display ATTRLIST and !ELEMENT in the outline. Write more tests with tolerant parser. |
Just wanted to confirm, did you mean 'ATTLIST' instead of 'ATTRLIST' or should parsing cover invalid tag names as well. We will need to implement handling invalid tag names which I can do in this pr. |
Yes I mean that.
Are you sure? For instance we have not that for element or attribute, why do we need that for ATTLIST? |
@angelozerr Sorry, I meant that if there was an invalid declaration. eg: |
654488e
to
1079ccf
Compare
with the current PR: <?xml version = "1.0"?>
<!DOCTYPE Folks [
<!ELEMENT Folks (Person*)>
<!ELEMENT Person (Name,Email?)>
<!ATTLIST Person Pin ID #REQUIRED>
<!ATTLIST Person Friend IDREF #IMPLIED>
<!ATTLIST Person Likes IDREFS #IMPLIED>
<!ELEMENT Name (#PCDATA)>
<!ELEMENT Email (#PCDATA)>
]>
<Folks>
<Person Pin="dd">
<Name></Name>
<Email></Email>
</Person>
</Folks> No outline is available. Furthermore, if you cut the all the
|
9dfe620
to
6e21e5d
Compare
I found that multiple attlists in one declaration are not supported, so only the 1st attribute shows in the outline eg. ant1.6.2.dtd (look at project, target): <?xml version="1.0" encoding="UTF-8" ?>
<!ENTITY % boolean "(true|false|on|off|yes|no)">
<!ENTITY % tasks "propertyfile | ccmkdir | importtypelib | vsscheckin | sql | cvspass | p4reopen | csc | dirname | wlrun | p4label | p4revert | replaceregexp | get | jjtree | sleep | jarlib-display | dependset | zip | patch | jspc | style | test | tstamp | unwar | vsshistory | icontract | cvschangelog | p4submit | ccmcheckin | p4change | bzip2 | sync | p4delete | vssadd | javadoc | p4integrate | translate | signjar | cclock | chown | vajload | jarlib-available | rexec | WsdlToDotnet | buildnumber | jpcovmerge | ejbjar | war | stlist | rename | sequential | serverdeploy | property | subant | move | ildasm | copydir | cccheckin | ccunlock | wljspc | fixcrlf | telnet | sosget | pathconvert | record | p4sync | exec | ccmklabel | p4edit | manifest | maudit | antlr | netrexxc | ftp | jpcovreport | execon | ccmcheckout | ant | xmlvalidate | xslt | p4resolve | iplanet-ejbc | ccmcheckintask | gzip | native2ascii | ccrmtype | starteam | ear | input | presetdef | rmic | checksum | mail | loadfile | vsscheckout | stylebook | soscheckin | mimemail | stlabel | gunzip | concat | cab | touch | parallel | splash | antcall | ccmkbl | cccheckout | typedef | p4have | filter | xmlproperty | import | copy | jsharpc | symlink | antstructure | script | ccmcreatetask | rpm | delete | replace | mmetrics | attrib | waitfor | untar | loadproperties | available | echoproperties | stcheckin | chgrp | vajexport | stcheckout | bunzip2 | whichresource | copyfile | p4labelsync | vsscreate | macrodef | ejbc | unjar | vbc | wsdltodotnet | mkdir | cvs | condition | tempfile | junitreport | ccmkattr | taskdef | echo | ccupdate | java | vsslabel | renameext | basename | javadoc2 | tar | vsscp | vajimport | p4fstat | setproxy | p4counter | wlstop | ilasm | soscheckout | apply | ccuncheckout | jarlib-resolve | jlink | cvstagdiff | javacc | chmod | pvcs | jarlib-manifest | jar | ccmklbtype | sound | scriptdef | defaultexcludes | mparse | blgenclient | uptodate | jjdoc | genkey | javah | ccmkelem | ccmreconfigure | fail | unzip | javac | p4add | jpcoverage | soslabel | depend | vssget | deltree | ddcreator | junit | sshexec | scp">
<!ENTITY % types "patternset | assertions | propertyset | filterset | libfileset | mergemapper | identitymapper | filterreader | unpackagemapper | scriptfilter | concatfilter | extension | fileset | dirset | globmapper | filelist | filterchain | path | compositemapper | classfileset | regexpmapper | selector | xmlcatalog | flattenmapper | description | chainedmapper | packagemapper | mapper | zipfileset | substitution | extensionSet | redirector | regexp">
<!ELEMENT project (target | extension-point | %tasks; | %types;)*>
<!ATTLIST project
name CDATA #IMPLIED
default CDATA #IMPLIED
basedir CDATA #IMPLIED>
<!ELEMENT target (%tasks; | %types;)*>
<!ATTLIST target
id ID #IMPLIED
name CDATA #REQUIRED
if CDATA #IMPLIED
unless CDATA #IMPLIED
depends CDATA #IMPLIED
extensionOf CDATA #IMPLIED
description CDATA #IMPLIED>
... |
@fbricon This is a symbols issue, all the values are there for this functionality to be implemented. |
ok, will open a separate bug then |
@NikolasKomonen you are right, it's an issue with symbol provider. I must fix that, but https://github.com/angelozerr/lsp4xml/blob/master/org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DTDAttlistDecl.java#L45 must be fixed too before to support list of attributes name instead of one attribute name. If you have time to do that, it should be cool, otherwise I will see it as soon as I find time. |
to follow up in #265 |
once this pr is merged I should be able to do a quickfix for #265 |
Great! I'm waiting for your "quickfix" to fixes the outline ATTRLIST |
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/DTDNotationDecl.java
Show resolved
Hide resolved
if (stream.skipWhitespace()) { | ||
return finishToken(offset, TokenType.Whitespace); | ||
} | ||
|
||
if (stream.advanceIfChar(_ORB)) { // ( | ||
nbBraceOpened++; | ||
state = ScannerState.DTDWithinElementContent; | ||
nbBraceOpened = 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nbBraceOpened++; seems safer, always accurate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed this because at this point it should always equal one. Before it relied on resetting the value back to 0.
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/dom/parser/XMLScanner.java
Show resolved
Hide resolved
org.eclipse.lsp4xml/src/main/java/org/eclipse/lsp4xml/services/XMLSymbolsProvider.java
Outdated
Show resolved
Hide resolved
6e21e5d
to
665be1f
Compare
If you try to format <!ATTRLIST name
<!ELEMENT name > you end up with <null
<null> |
So this is a blocker: in an empty dtd file, type Thread dump2018-12-18 01:01:28 Full thread dump Java HotSpot(TM) 64-Bit Server VM (25.141-b15 mixed mode):"ForkJoinPool.commonPool-worker-0" #29 daemon prio=5 os_prio=31 tid=0x00007fc67d2e9800 nid=0x100f waiting on condition [0x0000700005a87000] "ForkJoinPool.commonPool-worker-3" #28 daemon prio=5 os_prio=31 tid=0x00007fc67d0f0800 nid=0x5b0f runnable [0x0000700005983000] "ForkJoinPool.commonPool-worker-2" #27 daemon prio=5 os_prio=31 tid=0x00007fc67b924000 nid=0x9a0f runnable [0x0000700005880000] "Attach Listener" #20 daemon prio=9 os_prio=31 tid=0x00007fc67d26a800 nid=0x9d07 waiting on condition [0x0000000000000000] "pool-2-thread-1" #16 prio=5 os_prio=31 tid=0x00007fc67b8f2800 nid=0x5903 waiting on condition [0x000070000567b000] "DestroyJavaVM" #15 prio=5 os_prio=31 tid=0x00007fc67d002800 nid=0x1803 waiting on condition [0x0000000000000000] "pool-3-thread-1" #14 prio=5 os_prio=31 tid=0x00007fc67b8db800 nid=0x5803 runnable [0x0000700005578000] "pool-4-thread-1" #13 prio=5 os_prio=31 tid=0x00007fc67c93b800 nid=0x9f03 waiting on condition [0x0000700005475000] "Service Thread" #11 daemon prio=9 os_prio=31 tid=0x00007fc67c03c800 nid=0xa203 runnable [0x0000000000000000] "C1 CompilerThread2" #10 daemon prio=9 os_prio=31 tid=0x00007fc67c03c000 nid=0xa403 waiting on condition [0x0000000000000000] "C2 CompilerThread1" #9 daemon prio=9 os_prio=31 tid=0x00007fc67b832000 nid=0xa603 waiting on condition [0x0000000000000000] "C2 CompilerThread0" #8 daemon prio=9 os_prio=31 tid=0x00007fc67c04d800 nid=0xa803 waiting on condition [0x0000000000000000] "JDWP Event Helper Thread" #7 daemon prio=10 os_prio=31 tid=0x00007fc67c04c800 nid=0x5503 runnable [0x0000000000000000] "JDWP Transport Listener: dt_socket" #6 daemon prio=10 os_prio=31 tid=0x00007fc67c04c000 nid=0x3e07 runnable [0x0000000000000000] "Signal Dispatcher" #5 daemon prio=9 os_prio=31 tid=0x00007fc67c846000 nid=0x3d03 runnable [0x0000000000000000] "Surrogate Locker Thread (Concurrent GC)" #4 daemon prio=9 os_prio=31 tid=0x00007fc67c824000 nid=0x3b03 waiting on condition [0x0000000000000000] "Finalizer" #3 daemon prio=8 os_prio=31 tid=0x00007fc67b831800 nid=0x4603 in Object.wait() [0x0000700004a57000] "Reference Handler" #2 daemon prio=10 os_prio=31 tid=0x00007fc67b829000 nid=0x3503 in Object.wait() [0x0000700004954000] "VM Thread" os_prio=31 tid=0x00007fc67d02c800 nid=0x3403 runnable "Gang worker#0 (Parallel GC Threads)" os_prio=31 tid=0x00007fc67c803800 nid=0x1e07 runnable "Gang worker#1 (Parallel GC Threads)" os_prio=31 tid=0x00007fc67c812800 nid=0x2a03 runnable "Gang worker#2 (Parallel GC Threads)" os_prio=31 tid=0x00007fc67c813000 nid=0x2c03 runnable "Gang worker#3 (Parallel GC Threads)" os_prio=31 tid=0x00007fc67c814000 nid=0x5303 runnable "G1 Main Concurrent Mark GC Thread" os_prio=31 tid=0x00007fc67b80f000 nid=0x3103 runnable "Gang worker#0 (G1 Parallel Marking Threads)" os_prio=31 tid=0x00007fc67b810000 nid=0x4b03 runnable "G1 Concurrent Refinement Thread#0" os_prio=31 tid=0x00007fc67b807800 nid=0x4d03 runnable "G1 Concurrent Refinement Thread#1" os_prio=31 tid=0x00007fc67b807000 nid=0x3003 runnable "G1 Concurrent Refinement Thread#2" os_prio=31 tid=0x00007fc67c815000 nid=0x2f03 runnable "G1 Concurrent Refinement Thread#3" os_prio=31 tid=0x00007fc67b806000 nid=0x2e03 runnable "G1 Concurrent Refinement Thread#4" os_prio=31 tid=0x00007fc67c814800 nid=0x2d03 runnable "String Deduplication Thread" os_prio=31 tid=0x00007fc67c818800 nid=0x3303 runnable "VM Periodic Task Thread" os_prio=31 tid=0x00007fc67c03d800 nid=0xa103 waiting on condition JNI global references: 2535 |
threads state don't change even if you close the dtd |
It's again a problem with tolerant parser. You can see the trouble in the outline (nothing appears) |
The provided example: <!ATTRLIST name
<!ELEMENT name > has 'ATTLIST' spelt wrong. The ATTRLIST node is stored as a TextNode at the moment, that contains the start/end positions: |<!ATTRLIST name| I can modify this to make it work with the outline. |
Indeed @NikolasKomonen I have done this choice (store as DOMTextNode) the content of DTD which is not valid (to avoid loosing content when format is done even if content is not valid). But the problem here is that this DOMText breaks the parse of <!ELEMENT. I don't know if it's possible but we should improve DTD scanner to have more tolerant support. |
@angelozerr The parser doesnt break for this, I still get the document symbol for element and all the nodes are in DOMDocument. Both the DOMText and DTDElementDecl nodes are present. |
e376e4a
to
20cdda1
Compare
Fixes eclipse-lemminx#231 Signed-off-by: Nikolas Komonen <nikolaskomonen@gmail.com>
20cdda1
to
c7a78cc
Compare
@fbricon The tests are updated. |
Signed-off-by: Nikolas Komonen nikolaskomonen@gmail.com
Before adding unrecognizedParameters
Signed-off-by: Nikolas Komonen nikolaskomonen@gmail.com
asd