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

Update empty value in XML Variable substitution #6367

Merged
merged 1 commit into from
Feb 6, 2018

Conversation

vincent1173
Copy link
Contributor

No description provided.

@vincent1173 vincent1173 added Area: Release Area: AzureAppService Label to monitor Azure App Service issues labels Feb 6, 2018

if(xmlDomNode.getAttr(attributeName)) {
let isValueReplaced: boolean = false;
if(xmlDomNode.getAttr(attributeName) != undefined) {
Copy link

Choose a reason for hiding this comment

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

Does LTX provide method hasAttribute ?? If yes, we should use that method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesnt have hasAttributes.
Calling xmlDomNode.attrs provides the list of attributes present, from that we need to query for value attribute.
getAttr can be used to achieve the same result.

Copy link

Choose a reason for hiding this comment

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

I am not in favor of introducing isValueReplaced variable as the same thing can be achieved without that. ( Will discuss offline)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Planned to address the case, where the element has no value attribute and children with no value tag.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: AzureAppService Label to monitor Azure App Service issues Area: Release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants