1. Georg Brandl
  2. sphinx
  3. Pull requests

Pull requests

#99 Merged

Strip down seealso directives

  1. Robert Lehmann

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.