Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

support of comments in snippet #930

Merged
merged 2 commits into from
Nov 5, 2016

Conversation

pvojtechovsky
Copy link
Collaborator

The comments in snippet was always ignored. This PR includes this:

  • Test case which fails when comments are lost in snippet
  • Implementation of comments in snippet
  • new method CompilationUnit JDTSnippetCompiler.getSnippetCompilationUnit(), which returns CompilationUnit of the compilet snippet. Note: I had to remember the CompilationUnit of the snippet in the CompilationUnit cache, otherwise JDTCommentBuilder was not able to add comments. And at the end I removed that CU from the cache ... so it is now possible to access that CU from the JDTSnippetCompiler too.

It is the first chunk of changes suggested by PR #929

Copy link
Collaborator

@monperrus monperrus left a comment

Choose a reason for hiding this comment

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

thanks for the focused PR, it looks good, see comments

* @param filePath
* @return a cached compilation unit or null
*/
public CompilationUnit remove(String filePath) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename method to removeFromCache?

Copy link
Collaborator

Choose a reason for hiding this comment

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

package-visible instead of public?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removeFromCache

good idea

package-visible

It is different package ... or I do not know this java feature...

@pvojtechovsky
Copy link
Collaborator Author

I have committed the rename of method remove->removeFromCache. I do not know how to implement package visibility if the caller is in different package. I think it is not possible. But Travis does not want to check my last commit. OR am I wrong?

@monperrus monperrus merged commit 038c0e8 into INRIA:master Nov 5, 2016
@monperrus
Copy link
Collaborator

thanks!

@pvojtechovsky pvojtechovsky deleted the supportCommentsInSnippet branch November 5, 2016 21:57
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