#99 Merged
Repository
lehmannro/sphinx-simple-seealso sphinx-simple-seealso
Branch
default
Repository
birkenfeld/sphinx sphinx
Branch
default

Strip down seealso directives

Author
  1. Robert Lehmann avatarRobert Lehmann
Reviewers
Description

Instead of pull request #95, I'd propose simplifying seealso directives. They already spit out normal admonitions, they can also become a normal admonition directive.

This changes behaviour subtly in a few cases. The latex builder now says “See also” instead of “See Also,” the text builder now handles indentation differently, the HTML builder is missing the admonition-see-also CSS class.

Comments (6)

  1. Nozomu Kaneko

    FYI, I show a result of this change.

    For this source:

        .. seealso:: short text 1
    
        .. seealso::
    
           long text 1
    
        .. seealso:: short text 2
    
           long text 2
    

    the result before change (using the text builder) is here:

        See also:
    
           short text 1
    
        See also:
    
           long text 1
    
        See also:
    
           short text 2
    
           long text 2
    

    and the result after change is here:

        See also: short text 1
    
        See also: long text 1
    
        See also: short text 2
    
          long text 2
    

    My primary issue is the impact on the existing documents which use Sphinx.

  2. Robert Lehmann author

    Right, thanks for the reminder! There are subtle changes in other builders as well (see my first post.) I, personally, am OK with these, considering that all other admonitions are handled like that (the sample after the change) in the text builder as well. If we do not like that, we should change it right in the text builder.

    I didn't see any special cases in the original parser, but we should check against a larger corpus (say, the Python docs) if everything which has worked before continues to work properly.

  3. Georg Brandl repo owner

    I agree that consistency is preferable. This would not be a good change for a bugfix release, but for 1.2 it is fine. Robert, if you have time, would be nice to get this and the tests merged soon.

Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.