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

Already on GitHub? Sign in to your account

Add option to control displaying categories on menu #578

Merged
merged 1 commit into from Apr 1, 2013

Conversation

Projects
None yet
4 participants
Contributor

davidjb commented Nov 7, 2012

This option is DISPLAY_CATEGORIES_ON_MENU and controls, in both default themes, whether categories get displayed or not on the menu. I see this as being most useful in situations where one only wants a set listing of pages in the top menu and no categories at all.

This option is enabled by default, meaning a default install remains the same for backwards compatibility.

@almet almet and 1 other commented on an outdated diff Nov 16, 2012

tests/output/basic/archives.html
@@ -33,6 +33,7 @@
+
<li ><a href="./category/bar.html">bar</a></li>
@almet

almet Nov 16, 2012

Owner

why d owe have newlines here?

@davidjb

davidjb Nov 18, 2012

Contributor

Afaict, it's the introduction of the conditional statement for checking the option that adds this. I'll adjust the whitespace control and it should take this out.

Contributor

davidjb commented Nov 22, 2012

I've normalised whitespace after adding this new option - no additional whitespace is included after adding this option.

Owner

almet commented Nov 29, 2012

We have weird things going on with translations here, don't we?

Owner

justinmayer commented Feb 10, 2013

Hi David! Might you have time to rebase your commits into a single commit and ensure that it merges cleanly? I'm working through our pull request queue and would love to merge your contribution today, if possible. What do you think?

Contributor

davidjb commented Feb 10, 2013

@justinmayer Sure, no worries. Unsure if the translations and test issues will be resolved but I'll see what happens.

Owner

justinmayer commented Mar 4, 2013

Hi @davidjb. I'm going through the pull request queue and thought I'd check in. We'd like to release the next version of Pelican in the near future. Any chance you might be able to complete the work on this pull request so we can incorporate it? Feel free to refer to the Squashing Commits section of our Git Tips page.

Owner

justinmayer commented Mar 20, 2013

Hi David. Just checking in again. Anything I can do to help?

Contributor

davidjb commented Mar 21, 2013

Here we go. Rebased on the latest master, and unit tests pass with the regenerated example blog.

The whitespace changes in the output are due to to a different indent level for the pages on the menu now.

Owner

almet commented Mar 24, 2013

It seems we still have some translation-related changes here?

Owner

justinmayer commented Mar 24, 2013

David: This shows changes in the functional test output related to translations. Did you intend for those changes to occur? If so, can you explain the reason those values are changing?

Contributor

davidjb commented Mar 24, 2013

Changes aren't related to translations -- just adding the conditional statement into the base.html templates. This explains the small changes in whitespace in the output, but I can't explain the reason for the change in French -> English.

I followed these instructions: http://docs.getpelican.com/en/3.1.1/contribute.html#running-the-test-suite to run the test suite. After running this, for what it's worth, the tests do all pass on my system (and they did as well on Travis).

Thoughts?

Owner

justinmayer commented Mar 30, 2013

Hi David. Would you mind trying:

  1. Rebasing on current master
  2. Re-generating the test output
  3. Amend your commit with the new test output

In short, I suspect recent changes in master may eliminate the spurious translation output that currently appears in this pull request. If you need any help with this, please don't hesitate to reach out on the #pelican IRC channel on Freenode.

@davidjb davidjb Add new option for controlling whether to display categories on menu …
…or not

Conflicts:
	docs/changelog.rst
	pelican/tests/output/basic/tag/bar.html
	pelican/tests/output/basic/tag/baz.html
	pelican/tests/output/basic/tag/foo.html
	pelican/tests/output/custom/tag/bar.html
	pelican/tests/output/custom/tag/baz.html
	pelican/tests/output/custom/tag/foo.html
e042e11
Contributor

davidjb commented Apr 1, 2013

Okay, this looks better now.

Owner

justinmayer commented Apr 1, 2013

Excellent — merged. Thanks for the contribution and for patiently working with us to get everything squared away!

@justinmayer justinmayer added a commit that referenced this pull request Apr 1, 2013

@justinmayer justinmayer Merge pull request #578 from davidjb/display-categories-option
Add option to control displaying categories on menu
6726e5f

@justinmayer justinmayer merged commit 6726e5f into getpelican:master Apr 1, 2013

1 check passed

default The Travis build passed
Details

Coverage Status

Changes Unknown when pulling e042e11 on davidjb:display-categories-option into * on getpelican:master*.

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