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

Entity documentation has no value for entities declared with SYSTEM OR PUBLIC #746

Merged
merged 1 commit into from
Jun 1, 2020

Conversation

angelozerr
Copy link
Contributor

Entity documentation has no value for entities declared with SYSTEM OR PUBLIC

Fixes #741

Signed-off-by: azerr [email protected]

@angelozerr
Copy link
Contributor Author

This PR:

@xorye
Copy link

xorye commented May 29, 2020

Right now, it seems like this PR takes care of most of the issues. But the documentation for #741's case is different from the documentation for #742

741:
image

742:
image

To stay consistent, the #742's case should have SYSTEM ID instead of Value

I understand that this is a draft PR, so sorry if the implementation wasn't done

@angelozerr
Copy link
Contributor Author

To stay consistent, the #742's case should have SYSTEM ID instead of Value

Indeed it was a bug, it should be fixed.

I understand that this is a draft PR, so sorry if the implementation wasn't done

No please give me feedback and don't hesitate to give me those kind of comments.

It's draft PR because I have not written test and not commented my code.

@angelozerr angelozerr force-pushed the entity-system branch 6 times, most recently from 5435630 to 0723e3b Compare May 29, 2020 23:21
@angelozerr angelozerr marked this pull request as ready for review May 29, 2020 23:21
@angelozerr
Copy link
Contributor Author

Test are now written, please review it.

public static MarkupContent getDocumentation(DTDEntityDecl entity, EntityOriginType type, boolean markdown) {
String systemID = entity.getSystemId();
String publicID = entity.getPublicId();
String targetURI = entity.getNameParameter().getTargetURI();
Copy link

Choose a reason for hiding this comment

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

Do we need to check if entity.getNameParameter() == null here? Although, I'm not sure if it would happen in our case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If entity has name, entity.getNameParameter() is not null, so we don't need to check that.

@xorye
Copy link

xorye commented Jun 1, 2020

This PR looks & works great

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.

Entity documentation has no value for entities declared with SYSTEM OR PUBLIC
3 participants