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

Allow disabling footer date with includeDate template config (#910) #916

Merged
merged 1 commit into from
Feb 11, 2015
Merged

Allow disabling footer date with includeDate template config (#910) #916

merged 1 commit into from
Feb 11, 2015

Conversation

jviotti
Copy link

@jviotti jviotti commented Feb 9, 2015

As described in #910, this PR allows the user to disable the footer date in the default template, to prevent unnecessary noise in the commits.

This PR adds a new boolean option called includeDate to the templates config section.

Leaving it out preserves the current functionality (date is displayed). If you want to turn off that feature, you must pass includeDate: false explicitly:

{
    ...
    "templates": {
        ...
        "default": {
            "includeDate": false
        }
    },
    ...
}

I'll open an issue at https://github.com/jsdoc3/jsdoc3.github.com/issues along with this PR to include documentation about this new option.

@@ -29,7 +29,7 @@
<br class="clear">

<footer>
Documentation generated by <a href="https://github.com/jsdoc3/jsdoc">JSDoc <?js= env.version.number ?></a> on <?js= (new Date()) ?>
Documentation generated by <a href="https://github.com/jsdoc3/jsdoc">JSDoc<?js if(env.conf.templates.includeDate == undefined || env.conf.templates.includeDate) { ?><?js= env.version.number ?></a> on <?js= (new Date()) ?><?js } ?>
Copy link
Contributor

Choose a reason for hiding this comment

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

  • This code should be looking for the propertyenv.conf.templates.default.includeDate, because this option is specific to the default template.
  • For safety, check for env.conf.templates and env.conf.templates.default first.
  • Please check whether the property !== false, rather than for == undefined or truthy.
  • If you want to remove the JSDoc version number as well, please use a separate includeVersion property to control that behavior. (And please make sure the </a> tag shows up when this property is set to false. In your current patch, setting includeDate to false causes the </a> tag to be omitted.)

Copy link
Author

Choose a reason for hiding this comment

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

This code should be looking for the property env.conf.templates.default.includeDate, because this option is specific to the default template.

Indeed. I missed the default object you explained on the issue.

For safety, check for env.conf.templates and env.conf.templates.default first.

Included.

Please check whether the property !== false, rather than for == undefined or truthy.

Fixed, checking for false simplifies the condition.

If you want to remove the JSDoc version number as well, please use a separate includeVersion property to control that behavior. (And please make sure the tag shows up when this property is set to false. In your current patch, setting includeDate to false causes the tag to be omitted.)

My bad. I shouldn't be removing the version as well. Fixed.

hegemonic added a commit that referenced this pull request Feb 11, 2015
Allow disabling footer date with includeDate template config (#910)
@hegemonic hegemonic merged commit 8377c30 into jsdoc:master Feb 11, 2015
@hegemonic
Copy link
Contributor

Merged to master. I'll also merge the change to the 3.3.0 branch.

Thanks for fixing those issues and squashing the commits!

@jviotti jviotti deleted the 910-disable-footer-date branch February 11, 2015 12:19
@jviotti
Copy link
Author

jviotti commented Feb 11, 2015

@hegemonic Thanks, you're welcome, happy to have this issue fixed!

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.

None yet

2 participants