Skip to content

Optimize JsonAST.appendEscapedString character insertion#1706

Merged
Shadowfiend merged 1 commit into
lift:masterfrom
chriswebster:appendStringOp
Jul 3, 2015
Merged

Optimize JsonAST.appendEscapedString character insertion#1706
Shadowfiend merged 1 commit into
lift:masterfrom
chriswebster:appendStringOp

Conversation

@chriswebster
Copy link
Copy Markdown

Submitting a pull request based on mailing list discussion https://groups.google.com/forum/#!topic/liftweb/PD5GJYCBmx4 .

https://github.com/lift/framework/blob/master/contributors.md already contains the appropriate info.

@fmpwizard fmpwizard added this to the 3.0-M6 milestone May 30, 2015
@farmdawgnation
Copy link
Copy Markdown
Member

Can you please add yourself to the contributor file for me per the contributing guidelines? :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why do we need the if-statement? Is this part of ensuring the correct append is invoked?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

The isEmpty branch allows the character to be inserted directly instead of getting converted to a string, which incurs the additional overhead.

@chriswebster
Copy link
Copy Markdown
Author

I am currently on line 23 of the contributor agreement . https://github.com/lift/framework/blob/master/contributors.md#name-23

@Shadowfiend
Copy link
Copy Markdown
Member

I'm going to fix the formatting on the if statement and add a comment indicating what it's doing there, and then merge manually.

@Shadowfiend Shadowfiend merged commit 06d25c8 into lift:master Jul 3, 2015
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.

4 participants