Improved AMDJS merge implementation#149
Merged
yumaoka merged 4 commits intoIBM-Cloud:masterfrom Aug 23, 2019
Merged
Conversation
- Changed AMDJS merge implementation to use Rhino JS parser. Previous implementation used regex matcher for each line, and trying to replace value. This implementation was fragile and it does not work various different cases. - Previous merge operation tried to wrap long lines, but the new implementation put translated value in a single line. We may revisit this and re-implement line wrapping logic with the new code base. - Fixed IBM-Cloud#148 caused by the previous merge implementation. - Previous AMDJS filter write/merge implementation did not escape string literal properly - such as `\n` for line feed, `\"` for quoataion and so on (note: parser worked OK because Rhino parser does unescaping). Added proper string literal escape code. - Added new data drive test framework. With this framework, we can easily add parse/write/merge test cases without writing new JUnit test methods. - Deleted old AMDJS test case files not used.
Contributor
|
probably rebase on #150 to fix the build issue |
srl295
approved these changes
Aug 23, 2019
Contributor
srl295
left a comment
srl295
left a comment
There was a problem hiding this comment.
LGTM but let's get the ci to pass
| while (true) { | ||
| int tmp = brkItr.next(); | ||
| if (tmp == BreakIterator.DONE || tmp >= maxLen) { | ||
| static String escapeString(String str, Character quoteChar) { |
Contributor
There was a problem hiding this comment.
could this be done as part of gson?
Contributor
Author
There was a problem hiding this comment.
This is JS, not JSON. quote character might be quotation mark, or apostrophe.
There might be a standard library code to do this, but it's simple enough - so I just decided to implement it here.
Contributor
There was a problem hiding this comment.
fair enough and good point.
fix jdk versions
added 2 commits
August 23, 2019 16:59
- Changed AMDJS merge implementation to use Rhino JS parser. Previous implementation used regex matcher for each line, and trying to replace value. This implementation was fragile and it does not work various different cases. - Previous merge operation tried to wrap long lines, but the new implementation put translated value in a single line. We may revisit this and re-implement line wrapping logic with the new code base. - Fixed IBM-Cloud#148 caused by the previous merge implementation. - Previous AMDJS filter write/merge implementation did not escape string literal properly - such as `\n` for line feed, `\"` for quoataion and so on (note: parser worked OK because Rhino parser does unescaping). Added proper string literal escape code. - Added new data drive test framework. With this framework, we can easily add parse/write/merge test cases without writing new JUnit test methods. - Deleted old AMDJS test case files not used.
srl295
approved these changes
Aug 23, 2019
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
\nfor line feed,\"for quoataion and so on (note: parser worked OK because Rhino parser does unescaping). Added proper string literal escape code.