Skip to content

fix: fix incomplete behavior: Javadoc#setContent should parse tags#2765

Merged
pvojtechovsky merged 13 commits intoINRIA:masterfrom
monperrus:ref-comment
Nov 21, 2018
Merged

fix: fix incomplete behavior: Javadoc#setContent should parse tags#2765
pvojtechovsky merged 13 commits intoINRIA:masterfrom
monperrus:ref-comment

Conversation

@monperrus
Copy link
Copy Markdown
Collaborator

@monperrus monperrus commented Nov 13, 2018

Javadoc#setContent should parse tags

for this, encapsulates tag parsing in CtJavaDocImpl

(baby PR to make progress on #2172)

@monperrus monperrus changed the title WIP baby PR comment WIP fix: fix incomplete behavior: Javadoc#setContent should parse tags Nov 13, 2018
@monperrus
Copy link
Copy Markdown
Collaborator Author

Interesting, the tags.clear() surfaces a very subtle bug in cloning (testGenerateCloneVisitor in CI).

So we first have to look at #2767 and #2768.

@monperrus monperrus changed the title WIP fix: fix incomplete behavior: Javadoc#setContent should parse tags fix: fix incomplete behavior: Javadoc#setContent should parse tags Nov 16, 2018
String currentTagContent = "";
CtJavaDocTag.TagType currentTag = null;

String[] lines = cleanComment(content).split("\n|\r\n|\r");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

cleanComment always returns string with CtComment.LINE_SEPARATOR as line separator.
So it is enough to write `split("\n")

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.

fixed

CtJavaDocTag docTag = this.getFactory().Code().createJavaDocTag(tagContent, tagType);
this.addTag(docTag);
} else if (!tagContent.isEmpty()) {
this.setContent(tagContent.trim());
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Is it recursive call to setContent which calls defineCommentContent, which calls setContent ... ?
I have not checked it deeper I just do not understand it on first sight.

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.

fixed!

@pvojtechovsky
Copy link
Copy Markdown
Collaborator

Martin, it looks good to me.

@monperrus
Copy link
Copy Markdown
Collaborator Author

ready to merge.

@pvojtechovsky pvojtechovsky merged commit 8c00ba9 into INRIA:master Nov 21, 2018
@pvojtechovsky
Copy link
Copy Markdown
Collaborator

Thank you Martin!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants