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

Add a 'reverse' flag option to 'toctree' #2527

Merged
merged 1 commit into from Oct 12, 2016

Conversation

Projects
None yet
4 participants
@JohnVillalovos

JohnVillalovos commented May 5, 2016

I'm not sure the best way to do the unit tests. A little confused. So
help would be appreciated :)

Add the ability to reverse the list of entries when using 'toctree'.
This is likely most useful for people using the 'glob' flag option with
'toctree'

Change-Id: I1852479106c3d9c8253fd1625ced0077af0304e3

Add a 'reverse' flag option to 'toctree'
I'm not sure the best way to do the unit tests. A little confused. So
help would be appreciated :)

Add the ability to reverse the list of entries when using 'toctree'.
This is likely most useful for people using the 'glob' flag option with
'toctree'

Change-Id: I1852479106c3d9c8253fd1625ced0077af0304e3

@JohnVillalovos JohnVillalovos referenced this pull request May 5, 2016

Closed

Toctree reverse #971

@tk0miya tk0miya added the enhancement label May 6, 2016

@tk0miya tk0miya added this to the 1.5 milestone May 6, 2016

@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya May 6, 2016

Member

LGTM

Member

tk0miya commented May 6, 2016

LGTM

@@ -123,6 +123,16 @@ tables of contents. The ``toctree`` directive is the central element.
toctree directive. This is useful if you want to generate a "sitemap" from
the toctree.
You can use the ``reverse`` flag option to reverse the order of the entries
in the list. This can be useful when using the ``glob`` flag option to

This comment has been minimized.

@lehmannro

lehmannro May 6, 2016

Contributor

Minor nit: The rest of this file uses double spaces after period.

@lehmannro

lehmannro May 6, 2016

Contributor

Minor nit: The rest of this file uses double spaces after period.

@lehmannro

This comment has been minimized.

Show comment
Hide comment
@lehmannro

lehmannro May 6, 2016

Contributor

You can take a look at tests/test_toctree.py. It basically uses the HTML builder to then look at the relations between documents (up/next/previous). Writing a test for this PR would thus involve cloning tests/roots/test-glob-toctree into another testroot, enabling :reverse: and checking if the next/previous relationships are now reversed. You could maybe get away a little simpler with looking at the environment's toctrees (builder.env.toctree_includes).

Contributor

lehmannro commented May 6, 2016

You can take a look at tests/test_toctree.py. It basically uses the HTML builder to then look at the relations between documents (up/next/previous). Writing a test for this PR would thus involve cloning tests/roots/test-glob-toctree into another testroot, enabling :reverse: and checking if the next/previous relationships are now reversed. You could maybe get away a little simpler with looking at the environment's toctrees (builder.env.toctree_includes).

@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Aug 13, 2016

Member

Ping. any updates?
We will release major version in this autumn (now planning).

It would be nice if this PR will be included the release.

Member

tk0miya commented Aug 13, 2016

Ping. any updates?
We will release major version in this autumn (now planning).

It would be nice if this PR will be included the release.

@@ -46,6 +46,7 @@ class TocTree(Directive):
'includehidden': directives.flag,
'numbered': int_or_nothing,
'titlesonly': directives.flag,
'reverse': directives.flag,

This comment has been minimized.

@tk0miya

tk0miya Oct 12, 2016

Member

Which is better reverse or reversed ? I feel reversed is a little better.

@tk0miya

tk0miya Oct 12, 2016

Member

Which is better reverse or reversed ? I feel reversed is a little better.

@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Oct 12, 2016

Member

I added a testcases at https://github.com/tk0miya/sphinx/tree/2527_reversed_toctree .
As a test, I renamed the option to :reversed: in my branch.

If agreed, I would like to merge the branch. (if not, I'll pick only the testcodes)
Any comments?

Member

tk0miya commented Oct 12, 2016

I added a testcases at https://github.com/tk0miya/sphinx/tree/2527_reversed_toctree .
As a test, I renamed the option to :reversed: in my branch.

If agreed, I would like to merge the branch. (if not, I'll pick only the testcodes)
Any comments?

@shimizukawa

This comment has been minimized.

Show comment
Hide comment
@shimizukawa

shimizukawa Oct 12, 2016

Member

+1 for reversed to conform to other options like numbered.

Member

shimizukawa commented Oct 12, 2016

+1 for reversed to conform to other options like numbered.

@JohnVillalovos

This comment has been minimized.

Show comment
Hide comment
@JohnVillalovos

JohnVillalovos Oct 12, 2016

reversed sounds good to me too. Matches up with Python built-in function :)

Thanks for doing unit tests. I got confused on what to do.

JohnVillalovos commented Oct 12, 2016

reversed sounds good to me too. Matches up with Python built-in function :)

Thanks for doing unit tests. I got confused on what to do.

@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Oct 12, 2016

Member

@JohnVillalovos Thank you for comment. I will merge the change by my side later.

Member

tk0miya commented Oct 12, 2016

@JohnVillalovos Thank you for comment. I will merge the change by my side later.

@tk0miya tk0miya merged commit ca49c4c into sphinx-doc:master Oct 12, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@tk0miya

This comment has been minimized.

Show comment
Hide comment
@tk0miya

tk0miya Oct 12, 2016

Member

Merged.
Thank you for contribution!

Member

tk0miya commented Oct 12, 2016

Merged.
Thank you for contribution!

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