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

Change doc template to no print null missing values. #6565

Merged
merged 3 commits into from
Apr 22, 2020

Conversation

lbergelson
Copy link
Member

  • Now docs won't include null, "", or [] in the default value list.

This is one of the changes I mentioned on the call.

* Now docs won't include null, "", or [] in the default value list.
@lbergelson
Copy link
Member Author

Old:
Screen Shot 2020-04-22 at 1 04 46 PM
New:
Screen Shot 2020-04-22 at 1 02 50 PM

Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

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

Looks much cleaner, thanks. Merge when tests pass.

@lbergelson
Copy link
Member Author

Second commit changes:
Screen Shot 2020-04-22 at 1 11 46 PM

to this:
Screen Shot 2020-04-22 at 1 11 58 PM

@lbergelson
Copy link
Member Author

@cmnbroad I added another change too it, probably more little stuff we can do.

@lbergelson
Copy link
Member Author

It reports the argument ranges kind of weirdly, floats fault to -infinity -> infinity but so do ints and probably longs.

@@ -65,7 +65,7 @@
<#macro argumentDetails arg>
<hr style="border-bottom: dotted 1px #C0C0C0;" />
<h3><a name="${arg.name}">${arg.name} </a>
<#if arg.synonyms??> / <small>${arg.synonyms}</small></#if>
<#if arg.synonyms != "NA"> / <small>${arg.synonyms}</small></#if>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, this might not be safe without the "??" guard, which checks to see if the variable exists. If it doesn't (i.e, maybe positional args IIRC), this will throw.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ooh, ok, I'll add that check back as well.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks good now thx.

@lbergelson lbergelson merged commit 1bb8a03 into master Apr 22, 2020
@lbergelson
Copy link
Member Author

@cmnbroad We do have a bit of an awkward situation now with any argument that has "-NA" as a short name..

@lbergelson lbergelson deleted the lb_update_templates branch April 22, 2020 21:42
@cmnbroad
Copy link
Collaborator

True. Perhaps when we do the port to the new doclet api we should change the java code to always use null for missing data, and let the template decide how to present it. The we don't have to rely on string literal sentinels like this.

jonn-smith pushed a commit that referenced this pull request Jul 14, 2020
* Now docs won't include null, "", or [] in the default value list.
* If an argument has no short name it will no longer print NA in the short name options.
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