Skip to content
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

Infra: Use an option instead of argument in banner directives #3765

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Commits on Apr 28, 2024

  1. Use an option instead of argument in banner directives

    When a reST directive has `has_content` set to `True`, optional
    arguments and `final_argument_whitespace` set to True, the presence of a
    newline at the start of the directive is significant in communicating
    whether specific markup is treated as the argument or content.
    
        .. banner-1:: This is an argument
           This is still the argument.
    
           This is content.
    
        .. banner-2::
           This is still the argument.
    
           This is content.
    
        .. banner-3::
    
           This is content.
    
           This is more content.
    
    In the above example, `banner-2` and `banner-3` are very similar and
    only different in the presence of a newline. This is a subtle failure
    mode where written content can end up being silently ignored by the reST
    directive due to how arguments vs contents are parsed.
    
    Instead of accommodating for this behaviour by adding additional
    complexity to the directive being used, this change stops trying to mix
    multiline arguments and content in a single directive by using explicit
    options instead. This requires more verbosity but also eliminates this
    failure mode entirely.
    
        .. banner-1::
           :option: This is the option.
                    This is still the option.
    
        .. banner-2::
           This is content.
    
           This is still content.
    
        .. banner-3::
    
           This is content.
    
           This is still content.
    
    With this change, presence of a newline at the start of the directive is
    not determining if specific markup is treated as the argument or
    content. This is instead communicated by the presence of the option
    syntax which is more explicit and presents proper errors when the syntax
    is incorrect.
    pradyunsg committed Apr 28, 2024
    Configuration menu
    Copy the full SHA
    a8a4bfb View commit details
    Browse the repository at this point in the history

Commits on May 6, 2024

  1. Apply suggestions from code review

    Co-authored-by: Adam Turner <[email protected]>
    pradyunsg and AA-Turner committed May 6, 2024
    Configuration menu
    Copy the full SHA
    76b1aa3 View commit details
    Browse the repository at this point in the history