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

Allow variable tags to contain malformed xml #1193

Merged
merged 1 commit into from
Apr 12, 2020

Conversation

ang-zeyu
Copy link
Contributor

@ang-zeyu ang-zeyu commented Apr 11, 2020

What is the purpose of this pull request? (put "X" next to an item, remove the rest)

• [x] Bug fix

Fixes the first point mentioned here #1168 (comment)

What is the rationale for this request?
Simple patch to allow <variable> tags to hold any content built on #1047

Is there anything you'd like reviewers to focus on?
na

Testing instructions:
npm run test should pass

Proposed commit message: (wrap lines at 72 characters)
Allow variable tags to contain malformed xml

With the special tags patch, variables are able to contain custom
content possibly containing malformed xml.

Let’s add variable to the list of special tags to do so.

With the special tags patch, variables are able to contain custom
content possibly containing malformed xml.

Let’s add variable to the list of special tags to do so.
@nbriannl
Copy link
Contributor

nbriannl commented Apr 12, 2020

Thanks for this fix. Would anything regress now that variables can contain malformed xml? I don't see any reason why anything would regress. Test seem fine, update tests seem ok too.

Copy link
Contributor

@marvinchin marvinchin left a comment

Choose a reason for hiding this comment

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

LGTM

@ang-zeyu
Copy link
Contributor Author

Thanks for this fix. Would anything regress now that variables can contain malformed xml? I don't see any reason why anything would regress. Test seem fine, update tests seem ok too.

Yup, 2103 site had no diffs either. If anything it should fix edge cases of variables currently in use that had slightly incompatible syntax

@ang-zeyu ang-zeyu added this to the v2.13.2 milestone Apr 12, 2020
@ang-zeyu ang-zeyu merged commit bdc4b0c into MarkBind:master Apr 12, 2020
@damithc
Copy link
Contributor

damithc commented Apr 16, 2020

@ang-zeyu should we update the user guide somewhere to reflect this enhancement?

@ang-zeyu
Copy link
Contributor Author

@ang-zeyu should we update the user guide somewhere to reflect this enhancement?

Will do, didn't notice this limitation was documented

@damithc
Copy link
Contributor

damithc commented Apr 18, 2020

Tested in production. Good work @ang-zeyu and reviewers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants