[issue-78] EL expressions that contain unnecessary parentheses fail

[tmiyargi]

Added change from https://svn.apache.org/viewvc?view=revision&revision=1571242
Regenerated the parser and added a test.

[markt-asf]

The code this has been copied from is ALv2 licensed. Using ALv2 code comes with certain requirements around attribution that have not been met in this PR.
Also, the PR removes a license header from a file. Granted that file is generated and arguably doesn't need a header but I do not think it should be removed as part of this PR. If it is removed at all, it needs to be under a separate PR.

[markt-asf]

Changes requested as per my previous comment

[tmiyargi]

All changes are now in place, headers included and I have signed the Contributor Agreement.

[markt-asf]

I did obtain clarification that, for auto-generated files, the license of the output should be the same as the license for the input.

The additional licensing changes don't look right to me but I am not sufficiently well-versed in Eclipse procedures for pulling in ALv2 licensed content to determine if the proposed PR has done this correctly.

[bshannon]

@markt-asf It looks like you were the author of the original fix to the Apache code.
Why don't you just apply that fix, which you still own, to the EE4J code,
under the Eclipse license?

[tmiyargi]

@markt-asf @bshannon should I close this PR then so you can add the changes yourselves?

[bshannon]

@markt-asf, it's up to you. Can you just apply your fix, under the Eclipse license?

[DennisBayer]

Hi there,

any updates on this issue @markt-asf ? Additionally, is there any timeline for a release which would contain this fix?

Best regards
Dennis

[tmiyargi]

I've reworked the fix so it is no longer a copy. I hope you can merge it now.

@markt-asf has stopped responding

[bshannon]

It looks like @markt-asf has checked out of this project since he's not responding.
I've dismissed his review. We should consider removing him as a Committer.

The latest changes contain a lot of gratuitous formatting changes, is that because
these files were regenerated? Unfortunately, that makes it harder to review the
changes.

In any event, this needs review by someone more familiar with this code. I've added
some other reviewers who might be able to help.

[tmiyargi]

@bshannon Two files have been changed the others are regenerated using JavaCC. Thank you for looking into this.

[markt-asf]

My interest in EL at Eclipse is limited to the API.
Contributing to the implementation is not an itch I have any motivation to scratch.
The sole reason for my commenting on this PR was to point out that the original PR did not meet the requirements of the open source license of the project from which it was copied.

[bshannon]

The sole reason for my commenting on this PR was to point out that the original PR did not meet the requirements of the open source license of the project from which it was copied.

Which you could've solved by contributing your fix to this project as well.
That would've been simple for you to do. It's disappointing that you're
not willing to do so.

[tmiyargi]

Done rebase and solved conflicts, @arjantijms, @ruolli could you have a look?

[bmaxwell]

@arjantijms / @ruolli any updates on if this change is acceptable?

[rmartinc]

Hi,

The only real changes in this PR are in the following files:

• impl/src/main/java/com/sun/el/parser/ELParser.jjt
• src/test/java/org/glassfish/el/test/ELProcessorTest.java
• src/test/java/org/glassfish/el/test/LambdaTest.java
• src/test/java/org/glassfish/el/test/OperatorTest.java

All the other changes are result of executing the ant with the file impl/build.xml, which in turns executes JavaCC and regenerates the Java files for the EL parser. Maybe you feel more comfortable if only the modified files are included in the PR, the review will be easier. Later you can re-execute the ant and re-generate the auto-generated files by your own. WDYT?

[rmartinc]

Hi,

Sorry again for asking about this, but can we do something to move the PR forward? I proposed to send just one commit but maybe it's better if two commits are sent in the PR (the real modifications and then another one with the automatic ones). @tmiyargi cannot work more on the issue, so we'll need to send another PR for this.

Thanks!

[bshannon]

It seems that the people competent to review this (which doesn't include me) do not have the time to review this. I'm just going to approve and merge this, and we'll let the customers tell us if we got this wrong by filing bug reports.

Sorry this has taken so long.

[rmartinc]

Waiting for 3.0.4 relase then. Thanks a lot @bshannon !!!