Skip to content

improve string concatenation#1454

Closed
alexlamsl wants to merge 1 commit intomishoo:masterfrom
alexlamsl:binary-string
Closed

improve string concatenation#1454
alexlamsl wants to merge 1 commit intomishoo:masterfrom
alexlamsl:binary-string

Conversation

@alexlamsl
Copy link
Copy Markdown
Collaborator

@alexlamsl alexlamsl commented Jan 31, 2017

Born out of observation made in #301.

Tests in #1453 will need to update for more optimal results after this PR merges.

lib/compress.js Outdated
|| (self.operator == "+"
&& (self.right.left.is_string(compressor)
|| (self.left.is_string(compressor)
&& self.right.right.is_string(compressor))))))
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

@kzc this practical joke has gotten seriously out of hand... any suggestions on improving readability would be greatly appreciated.

Copy link
Copy Markdown
Contributor

@kzc kzc Jan 31, 2017

Choose a reason for hiding this comment

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

       if (self.right instanceof AST_Binary
           && self.right.operator == self.operator
           && (self.operator == "&&"
               || self.operator == "||"
               || (self.operator == "+"
                   && (self.right.left.is_string(compressor)
                       || (self.left.is_string(compressor)
                           && self.right.right.is_string(compressor))))))
       {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks!

1 + 2 + ("3" + 4 + 5),
1 + 2 + ("3" + 4 + "5"),
1 + 2 + ("3" + "4" + 5),
1 + 2 + ("3" + "4" + "5"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Ideally we don't want the test to be too long so it's easier to compare input: with expected:.

I'd break this concat_3 into four smaller tests.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Done.

1 + "2" + ("3" + 4 + 5),
1 + "2" + ("3" + 4 + "5"),
1 + "2" + ("3" + "4" + 5),
1 + "2" + ("3" + "4" + "5"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

new test

"1" + 2 + ("3" + 4 + 5),
"1" + 2 + ("3" + 4 + "5"),
"1" + 2 + ("3" + "4" + 5),
"1" + 2 + ("3" + "4" + "5"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

new test

@kzc
Copy link
Copy Markdown
Contributor

kzc commented Feb 1, 2017

LGTM

shuffle associative operations to minimise parentheses and aid other uglification efforts
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Feb 11, 2017
@alexlamsl alexlamsl mentioned this pull request Feb 11, 2017
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Feb 11, 2017
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this pull request Feb 18, 2017
shuffle associative operations to minimise parentheses and aid other uglification efforts

closes mishoo#1454
@alexlamsl alexlamsl closed this in 6b3c49e Feb 23, 2017
@alexlamsl alexlamsl deleted the binary-string branch February 24, 2017 00:20
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.

2 participants