Skip to content
This repository was archived by the owner on Apr 18, 2023. It is now read-only.

Fix simple content extension.#47

Merged
pleonex merged 1 commit intopleonex:masterfrom
ph777:fix/simple-content-extension
Jun 4, 2018
Merged

Fix simple content extension.#47
pleonex merged 1 commit intopleonex:masterfrom
ph777:fix/simple-content-extension

Conversation

@ph777
Copy link

@ph777 ph777 commented May 26, 2018

This PR extends autocomplete-xml package with ability to recognize simple content extension. For example when a simple type is extended with an attribute.

<xs:simpleType name="extendedType">
  <xs:extension base="xs:string">
    <xs:attribute name="myAttr" type="xs:string"/>
  </xs:extension>
</xs:simpleType>

The fix shoud work correcly for both standard simple types like xs:string and custom simples, e.g. string restrictions, etc.

Please review the PR and consider merging it to your package.

Copy link
Owner

@pleonex pleonex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes!

else if node.complexContent?[0].extension
type.xsdChildrenMode = 'extension'
type.xsdChildren = node.complexContent[0].extension[0]
else if node.simpleContent?[0].extension
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we parsing the simpleType node in parseComplexType? I think that node is only parsed in parseSimpleType, right?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are not parsing simple type, but complex type with simple content. The simpleContent element can appear in complex type, e.g. when defining an element with string content (simple content) and an attribute:

<xs:complexType name="myType">
    <xs:simpleContent>
        <xs:extension base="xs:string">
            <xs:attribute name="name" type="xs:string" use="required"/>
        </xs:extension>
    </xs:simpleContent>
</xs:complexType>

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it makes sense now, thanks!

atom.notifications.addError "can't find base type " + extenType.$.base
continue
# Ingore link type for simple types reffering to standard simple types like xs:string, etc.
if type.xsdType == 'simple' and ":" in extenType.$.base
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about creating a global list of built-in types? Then we can check here against that list (and also in #48). As a start we can add to the list the most common built-in types:

  • xs:string
  • xs:decimal
  • xs:integer
  • xs:boolean
  • xs:date
  • xs:time

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the way I distinguish between a built-in type and custom type is not perfect, but it is working fine for the usual cases. So I think it is a reasonable simplification.

If you want to include list of built-in types than I would suggest to make the list complete. To be perfect the namespace prefix would have to be checked too to distinguish between xs:string and my:string.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense, it will be implemented from a different issue from #49.

@pleonex pleonex merged commit 9ad516b into pleonex:master Jun 4, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants