ionelmc / python-darkslide

Generate HTML5 slideshows from markdown, ReST, or textile
http://ionelmc.github.io/python-darkslide/
Apache License 2.0
97 stars 22 forks source link

TOC contains empty entries #5

Open ptitmain opened 6 years ago

ptitmain commented 6 years ago

Hi, I just tried python-darkslide and it seems that it contains a bug I have patched years ago in the landslide project but that I forgot to upstream. When increasing the hierarchy of the document, entries with high level generate an empty entry in the TOC. The code involved is:

            if slide_vars['level'] and slide_vars['level'] <= TOC_MAX_LEVEL:
                self.add_toc_entry(slide_vars['title'], slide_vars['level'],
                                   slide_number)
            else:
                # Put something in the TOC even if it doesn't have a title or level
               self.add_toc_entry(u"-", 1, slide_number)

And the patch I did: https://github.com/ptitmain/landslide/commit/e9d4dbdf798ad1f2728162c090ddfeb5c7385048

Is this patch ok ? If yes, can darkslide include such a patch ? Best regards.

ionelmc commented 6 years ago

Hmmm, perhaps we could have a better replacement? How about "(no title)"? What do you usually put in a slide without title?

ionelmc commented 6 years ago

Another way is to leave it empty ("&nbsp;") - I think I like that one best. Eg: image

ptitmain commented 6 years ago

Ok, I have produced a MWE. The bugs seems to appear when a "----" is inserted between the first section and the first subsection: the 4th level of section appears empty in the TOC when hitting "t"...

=====================
Développement Android
=====================

.. |year| date:: %Y

.. sectnum::
   :depth: 2

Title page

.. The bug is due to the following break:

----

--------------
Le SDK Android
--------------

.. contents::
   :local:
   :depth: 1

----

Android
-------

Android test

----

L'Operating System
~~~~~~~~~~~~~~~~~~

OS Test

----

Historique des versions
~~~~~~~~~~~~~~~~~~~~~~~

Histo Test

selection_082

ionelmc commented 6 years ago

Looks like this here: image

What version of docutils you have?

ptitmain commented 6 years ago

Sorry, wrong copy paste (one level more is needed):

=====================
Développement Android
=====================

.. |year| date:: %Y

.. sectnum::
   :depth: 2

Title page

.. The bug is due to the following break:

----

--------------
Le SDK Android
--------------

.. contents::
   :local:
   :depth: 1

----

Android
-------

Android test

----

L'Operating System
~~~~~~~~~~~~~~~~~~

OS Test

----

Historique des versions
~~~~~~~~~~~~~~~~~~~~~~~

Histo Test
ptitmain commented 6 years ago

docutils 0.12

ii docutils-common 0.12+dfsg-1

ionelmc commented 6 years ago

I see the problem, there's only support for 2 levels (TOC_MAX_LEVEL). It can be fixed.

ptitmain commented 6 years ago

Ok. Thus, probably the TOC should ignore other levels... or support more levels... In my opinion, there should be a parameter to control the TOC depth from the config file... (I see the variable in generator.py)

ionelmc commented 6 years ago

I guess we could change it to 6 (as <h6> is max) and fix base.html/screen.css to support the extra nesting. You wanna do it?

We'd need to use a jinja macro to adequately support the variable nesting in base.html (eg: http://jinja.pocoo.org/docs/2.9/templates/#macros).

ptitmain commented 6 years ago

Ok, I'll try to do it.

ptitmain commented 6 years ago

I am working on a patch. Still wondering what is the goal of the else branch:

            if slide_vars['level'] and slide_vars['level'] <= TOC_MAX_LEVEL:
                self.add_toc_entry(slide_vars['title'], slide_vars['level'],
                                   slide_number)
            else:
                # Put something in the TOC even if it doesn't have a title or level
                self.add_toc_entry(u"-", 1, slide_number)

In what case do we need an entry in the toc if there is no specified level ?

ionelmc commented 6 years ago

I think the placeholder is there so you can navigate to the titleless slide via toc. I guess the else branch could be removed if the slide numbers are filled into the parent slide, eg: image