Retaining of comments and their position in the actual code after parsing. #465
Conversation
statement like c = a + b //test
+ d
Added pom.xml for WM to generate jar and push to repository.
Added minimum Java version requirement for compilation
- Handling of inline comments for "do" satement in a do-while
- Modified toSource methods of do and while AstNodes for handling of inline comments.
Handling of String literals if they are enclosed within backtick (`)
- Handling of inline comments if used after try, with, case
- adding messages resources to rhino.jar itself.
- Inline comments at the case level are handled properly
- Inline comments within object literal are ignored.
- Fix below scenario
if(a == 3) {
b = 3;
}
//inlinecomment 1
else {
b = 5;
}
- Fix for a bug in function with no argument after last comma
f1(a,b,);
- Added a setter method for updating the comment text.
- Modified minor version to avoid manual copy of new jar.
- Modified minor version to avoid manual copy of new jar. because in previous build, resource files are not copied.
- ignoring comments if they are placed after an incomplete statement like var a = 3 /* */ var t=3;
|
Thanks for doing this -- it looks like a cool use of Rhino. Since this PR makes a ton of changes to Parser, which is pretty important, I wanted to make sure that we were careful with it. I see changes in here that affect comments in a lot of places -- while loops, for loop, if statements, etc. However, the only new test case is pretty trivial. How do we ensure going forward that all the different types of comments here are supported? Would it be possible to make the test suite for this PR more extensive? Also, it looks from the test changes that consumers of the AST will have to change as a result of this PR. Does this only affect use cases in which the flag is set in CompilerEnvirons? I just want to understand the impact of this PR on backward compatibility, and put it clearly in the commit messages and eventually the release notes. Finally, would it be possible to rebase and squash this into fewer commits? This isn't a hard and fast rule but when I see commits like "remove pom file" then I'd rather squash a PR and get those out of the Rhino commit history. Thansk! |
|
Thanks for the comments. This changes will not effect backward Compatibility as this will be executed only if "recordingComments" of "CompilerEnvirons" is set. I'll make below changes and then reply.
|
statement like c = a + b //test
+ d
Added pom.xml for WM to generate jar and push to repository.
Added minimum Java version requirement for compilation
- Handling of inline comments for "do" satement in a do-while
- Modified toSource methods of do and while AstNodes for handling of inline comments.
Handling of String literals if they are enclosed within backtick (`)
- Handling of inline comments if used after try, with, case
- adding messages resources to rhino.jar itself.
- Inline comments at the case level are handled properly
- Inline comments within object literal are ignored.
- Fix below scenario
if(a == 3) {
b = 3;
}
//inlinecomment 1
else {
b = 5;
}
- Fix for a bug in function with no argument after last comma
f1(a,b,);
- Added a setter method for updating the comment text.
- Modified minor version to avoid manual copy of new jar.
- Modified minor version to avoid manual copy of new jar. because in previous build, resource files are not copied.
- ignoring comments if they are placed after an incomplete statement like
var a = 3
/* */ var t=3;
- Ignoring inline comments which come within an expression or statement
like c = a + b //test + d
- Handling of inline comments for "do" satement in a do-while
- Modified toSource methods of do and while AstNodes for handling of
inline comments.
- Handling of String literals if they are enclosed within backtick (`)
- Handling of inline comments if used after try, with, case - adding
messages resources to rhino.jar itself.
- Inline comments at the case level are handled properly
- Inline comments within object literal are ignored.
- Fix below scenario
if(a == 3) {
b = 3;
} //inlinecomment 1
else {
b = 5;
}
- Fix for a bug in function with no argument after last comma.
f1(a,b,);
- Added a setter method for updating the comment text.
|
Thank you!
I'd be happy with "more extensive" test cases ;-) I saw that you were
supporting comments in a number of different places in the source and I
just want to make sure that we have a test for each one.
…On Sun, Aug 5, 2018 at 10:09 PM Ravi Kishore ***@***.***> wrote:
Thanks for the comments.
This changes will not effect backward Compatibility as this will be
executed only if "recordingComments" of "CompilerEnvirons" is set.
I'll make below changes and then reply.
1. Rebase and squash with fewer commits.
2. Write extensive test cases for this PR.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#465 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAf0a7tK4Qj55R7gVnpKWxvmUnTs2mYtks5uN899gaJpZM4Vao_b>
.
|
- Added test cases testing inline comments of if, for, loops
|
Hi Greg Brail, |
|
I think we still have some flaky tests in the suite -- I re-ran the Travis job and it passed. So I think you're good to go from a test perspective. |
|
Thank you Greg Brail. |
|
This looks good. I'm going to squash it because it's got a bunch of "remove pom.xml" kinds of commits. |
|
Thanks Greg Brail. |
I wanted to use rhino parser to generate a new js file from existing js file after some modifications to some methods or variable names etc. for example as below
old JS file:
`Application.run(function ($rootScope) {
"use strict";
/* perform any action on the variables within this block(on-page-load) /
$rootScope.onAppVariablesReady = function () {
/
* variables can be accessed through '$rootScope.Variables' property here
* e.g. $rootScope.Variables.staticVariable1.getData()
*/
};
});`
Post Refactoring:
/* perform any action on the variables within this block(on-page-load) */ App.onAppVariablesReady = function () { /* * variables can be accessed through '$rootScope.Variables' property here * e.g. $rootScope.Variables.staticVariable1.getData() */ };Using rhino parser I was able to achieve this but comments are lost. So providing a fix such that if "CompilerEnvirons.setRecordingComments(true)" then comments are added to the AST.