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

Fix: no trailing comma in multi-line signatures by default #12975

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
2 changes: 2 additions & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -18,5 +18,7 @@ Features added
Bugs fixed
----------

* #12975: No trailing comma in C/C++ multi-line signatures
TLouf marked this conversation as resolved.
Show resolved Hide resolved

Testing
-------
18 changes: 18 additions & 0 deletions doc/usage/configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -4040,6 +4040,15 @@ Options for the Javascript domain

.. versionadded:: 7.1

.. confval:: javascript_trailing_comma_in_multi_line_signatures
TLouf marked this conversation as resolved.
Show resolved Hide resolved
:type: :code-py:`bool`
:default: :code-py:`True`

A boolean that decides whether to add a trailing comma to
parameter lists spanning multiple lines.

.. versionadded:: 8.2


Options for the Python domain
-----------------------------
Expand Down Expand Up @@ -4129,6 +4138,15 @@ Options for the Python domain

.. versionadded:: 7.1

.. confval:: python_trailing_comma_in_multi_line_signatures
TLouf marked this conversation as resolved.
Show resolved Hide resolved
:type: :code-py:`bool`
:default: :code-py:`True`

A boolean that decides whether to add a trailing comma to
parameter lists spanning multiple lines.

.. versionadded:: 8.2

.. confval:: python_use_unqualified_type_names
:type: :code-py:`bool`
:default: :code-py:`False`
Expand Down
6 changes: 4 additions & 2 deletions sphinx/addnodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,8 @@ class desc_parameterlist(nodes.Part, nodes.Inline, nodes.FixedTextElement):

As default the parameter list is written in line with the rest of the signature.
Set ``multi_line_parameter_list = True`` to describe a multi-line parameter list.
In that case each parameter will then be written on its own, indented line.
In that case each parameter will then be written on its own, indented line. A
trailing comma will be added on the last line if ``trailing_comma`` is set to True.
"""

child_text_separator = ', '
Expand All @@ -257,7 +258,8 @@ class desc_type_parameter_list(nodes.Part, nodes.Inline, nodes.FixedTextElement)

As default the type parameters list is written in line with the rest of the signature.
Set ``multi_line_parameter_list = True`` to describe a multi-line type parameters list.
In that case each type parameter will then be written on its own, indented line.
In that case each type parameter will then be written on its own, indented line. A
trailing comma will be added on the last line if ``trailing_comma`` is set to True.
"""

child_text_separator = ', '
Expand Down
12 changes: 11 additions & 1 deletion sphinx/domains/javascript.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,8 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
and (len(sig) > max_len > 0)
)

trailing_comma = self.env.config.javascript_trailing_comma_in_multi_line_signatures

display_prefix = self.get_display_prefix()
if display_prefix:
signode += addnodes.desc_annotation('', '', *display_prefix)
Expand All @@ -128,7 +130,12 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
if not arglist:
signode += addnodes.desc_parameterlist()
else:
_pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
_pseudo_parse_arglist(
signode,
arglist,
multi_line_parameter_list,
trailing_comma,
)
return fullname, prefix

def _object_hierarchy_parts(self, sig_node: desc_signature) -> tuple[str, ...]:
Expand Down Expand Up @@ -505,6 +512,9 @@ def setup(app: Sphinx) -> ExtensionMetadata:
app.add_config_value(
'javascript_maximum_signature_line_length', None, 'env', {int, type(None)},
)
app.add_config_value(
'javascript_trailing_comma_in_multi_line_signatures', True, 'env',
)
TLouf marked this conversation as resolved.
Show resolved Hide resolved
return {
'version': 'builtin',
'env_version': 3,
Expand Down
3 changes: 3 additions & 0 deletions sphinx/domains/python/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -935,6 +935,9 @@ def setup(app: Sphinx) -> ExtensionMetadata:
app.add_config_value(
'python_maximum_signature_line_length', None, 'env', {int, type(None)},
)
app.add_config_value(
'python_trailing_comma_in_multi_line_signatures', True, 'env',
TLouf marked this conversation as resolved.
Show resolved Hide resolved
)
app.add_config_value('python_display_short_literal_types', False, 'env')
app.connect('object-description-transform', filter_meta_fields)
app.connect('missing-reference', builtin_resolver, priority=900)
Expand Down
12 changes: 10 additions & 2 deletions sphinx/domains/python/_annotations.py
Original file line number Diff line number Diff line change
Expand Up @@ -365,10 +365,12 @@ def _pformat_token(self, tok: Token, native: bool = False) -> str:
def _parse_type_list(
tp_list: str, env: BuildEnvironment,
multi_line_parameter_list: bool = False,
trailing_comma: bool = True,
) -> addnodes.desc_type_parameter_list:
"""Parse a list of type parameters according to PEP 695."""
type_params = addnodes.desc_type_parameter_list(tp_list)
type_params['multi_line_parameter_list'] = multi_line_parameter_list
type_params['trailing_comma'] = trailing_comma
# formal parameter names are interpreted as type parameter names and
# type annotations are interpreted as type parameter bound or constraints
parser = _TypeParameterListParser(tp_list)
Expand Down Expand Up @@ -425,11 +427,14 @@ def _parse_type_list(


def _parse_arglist(
arglist: str, env: BuildEnvironment, multi_line_parameter_list: bool = False,
arglist: str, env: BuildEnvironment,
multi_line_parameter_list: bool = False,
trailing_comma: bool = True,
) -> addnodes.desc_parameterlist:
"""Parse a list of arguments using AST parser"""
params = addnodes.desc_parameterlist(arglist)
params['multi_line_parameter_list'] = multi_line_parameter_list
params['trailing_comma'] = trailing_comma
sig = signature_from_str('(%s)' % arglist)
last_kind = None
for param in sig.parameters.values():
Expand Down Expand Up @@ -478,7 +483,9 @@ def _parse_arglist(


def _pseudo_parse_arglist(
signode: desc_signature, arglist: str, multi_line_parameter_list: bool = False,
signode: desc_signature, arglist: str,
multi_line_parameter_list: bool = False,
trailing_comma: bool = True,
) -> None:
""""Parse" a list of arguments separated by commas.

Expand All @@ -488,6 +495,7 @@ def _pseudo_parse_arglist(
"""
paramlist = addnodes.desc_parameterlist()
paramlist['multi_line_parameter_list'] = multi_line_parameter_list
paramlist['trailing_comma'] = trailing_comma
stack: list[Element] = [paramlist]
try:
for argument in arglist.split(','):
Expand Down
17 changes: 13 additions & 4 deletions sphinx/domains/python/_object.py
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
and (sig_len - (arglist_span[1] - arglist_span[0])) > max_len > 0
)

trailing_comma = self.env.config.python_trailing_comma_in_multi_line_signatures
sig_prefix = self.get_signature_prefix(sig)
if sig_prefix:
if type(sig_prefix) is str:
Expand All @@ -277,25 +278,33 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]

if tp_list:
try:
signode += _parse_type_list(tp_list, self.env, multi_line_type_parameter_list)
signode += _parse_type_list(
tp_list, self.env, multi_line_type_parameter_list, trailing_comma
TLouf marked this conversation as resolved.
Show resolved Hide resolved
)
except Exception as exc:
logger.warning("could not parse tp_list (%r): %s", tp_list, exc,
location=signode)

if arglist:
try:
signode += _parse_arglist(arglist, self.env, multi_line_parameter_list)
signode += _parse_arglist(
arglist, self.env, multi_line_parameter_list, trailing_comma
TLouf marked this conversation as resolved.
Show resolved Hide resolved
)
except SyntaxError:
# fallback to parse arglist original parser
# (this may happen if the argument list is incorrectly used
# as a list of bases when documenting a class)
# it supports to represent optional arguments (ex. "func(foo [, bar])")
_pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
_pseudo_parse_arglist(
signode, arglist, multi_line_parameter_list, trailing_comma
TLouf marked this conversation as resolved.
Show resolved Hide resolved
)
except (NotImplementedError, ValueError) as exc:
# duplicated parameter names raise ValueError and not a SyntaxError
logger.warning("could not parse arglist (%r): %s", arglist, exc,
location=signode)
_pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
_pseudo_parse_arglist(
signode, arglist, multi_line_parameter_list, trailing_comma
TLouf marked this conversation as resolved.
Show resolved Hide resolved
)
else:
if self.needs_arglist():
# for callables, add an empty parameter list
Expand Down
21 changes: 15 additions & 6 deletions sphinx/writers/html5.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ def _visit_sig_parameter_list(
self.required_params_left = sum(self.list_is_required_param)
self.param_separator = node.child_text_separator
self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
self.trailing_comma = node.get('trailing_comma', False)
if self.multi_line_parameter_list:
self.body.append('\n\n')
self.body.append(self.starttag(node, 'dl'))
Expand Down Expand Up @@ -239,7 +240,8 @@ def depart_desc_parameter(self, node: Element) -> None:
or is_required
and (is_last_group or next_is_required)
):
self.body.append(self.param_separator)
if not is_last_group or opt_param_left_at_level or self.trailing_comma:
self.body.append(self.param_separator)
self.body.append('</dd>\n')

elif self.required_params_left:
Expand Down Expand Up @@ -282,19 +284,26 @@ def visit_desc_optional(self, node: Element) -> None:

def depart_desc_optional(self, node: Element) -> None:
self.optional_param_level -= 1
level = self.optional_param_level
if self.multi_line_parameter_list:
# If it's the first time we go down one level, add the separator
# before the bracket.
if self.optional_param_level == self.max_optional_param_level - 1:
max_level = self.max_optional_param_level
len_lirp = len(self.list_is_required_param)
is_last_group = self.param_group_index + 1 == len_lirp
# If it's the first time we go down one level, add the separator before the
# bracket, except if this is the last parameter and the parameter list
# should not feature a trailing comma.
if level == max_level - 1 and (
not is_last_group or level > 0 or self.trailing_comma
):
self.body.append(self.param_separator)
self.body.append('<span class="optional">]</span>')
# End the line if we have just closed the last bracket of this
# optional parameter group.
if self.optional_param_level == 0:
if level == 0:
self.body.append('</dd>\n')
else:
self.body.append('<span class="optional">]</span>')
if self.optional_param_level == 0:
if level == 0:
self.param_group_index += 1

def visit_desc_annotation(self, node: Element) -> None:
Expand Down
16 changes: 12 additions & 4 deletions sphinx/writers/latex.py
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,7 @@ def _visit_sig_parameter_list(
self.required_params_left = sum(self.list_is_required_param)
self.param_separator = r'\sphinxparamcomma '
self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
self.trailing_comma = node.get('trailing_comma', False)

def visit_desc_parameterlist(self, node: Element) -> None:
if self.has_tp_list:
Expand Down Expand Up @@ -1015,7 +1016,7 @@ def _depart_sig_parameter(self, node: Element) -> None:
if (
opt_param_left_at_level
or is_required
and (is_last_group or next_is_required)
and (next_is_required or self.trailing_comma)
):
self.body.append(self.param_separator)

Expand Down Expand Up @@ -1057,13 +1058,20 @@ def visit_desc_optional(self, node: Element) -> None:

def depart_desc_optional(self, node: Element) -> None:
self.optional_param_level -= 1
level = self.optional_param_level
if self.multi_line_parameter_list:
max_level = self.max_optional_param_level
len_lirp = len(self.list_is_required_param)
is_last_group = self.param_group_index + 1 == len_lirp
# If it's the first time we go down one level, add the separator before the
# bracket.
if self.optional_param_level == self.max_optional_param_level - 1:
# bracket, except if this is the last parameter and the parameter list
# should not feature a trailing comma.
if level == max_level - 1 and (
not is_last_group or level > 0 or self.trailing_comma
):
self.body.append(self.param_separator)
self.body.append('}')
if self.optional_param_level == 0:
if level == 0:
self.param_group_index += 1

def visit_desc_annotation(self, node: Element) -> None:
Expand Down
19 changes: 14 additions & 5 deletions sphinx/writers/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -646,6 +646,7 @@ def _visit_sig_parameter_list(
self.required_params_left = sum(self.list_is_required_param)
self.param_separator = ', '
self.multi_line_parameter_list = node.get('multi_line_parameter_list', False)
self.trailing_comma = node.get('trailing_comma', False)
if self.multi_line_parameter_list:
self.param_separator = self.param_separator.rstrip()
self.context.append(sig_close_paren)
Expand Down Expand Up @@ -697,7 +698,8 @@ def visit_desc_parameter(self, node: Element) -> None:
or is_required
and (is_last_group or next_is_required)
):
self.add_text(self.param_separator)
if not is_last_group or opt_param_left_at_level or self.trailing_comma:
self.add_text(self.param_separator)
self.end_state(wrap=False, end=None)

elif self.required_params_left:
Expand Down Expand Up @@ -738,20 +740,27 @@ def visit_desc_optional(self, node: Element) -> None:

def depart_desc_optional(self, node: Element) -> None:
self.optional_param_level -= 1
level = self.optional_param_level
if self.multi_line_parameter_list:
max_level = self.max_optional_param_level
len_lirp = len(self.list_is_required_param)
is_last_group = self.param_group_index + 1 == len_lirp
# If it's the first time we go down one level, add the separator before the
# bracket.
if self.optional_param_level == self.max_optional_param_level - 1:
# bracket, except if this is the last parameter and the parameter list
# should not feature a trailing comma.
if level == max_level - 1 and (
not is_last_group or level > 0 or self.trailing_comma
):
self.add_text(self.param_separator)
self.add_text(']')
# End the line if we have just closed the last bracket of this group of
# optional parameters.
if self.optional_param_level == 0:
if level == 0:
self.end_state(wrap=False, end=None)

else:
self.add_text(']')
if self.optional_param_level == 0:
if level == 0:
self.param_group_index += 1

def visit_desc_annotation(self, node: Element) -> None:
Expand Down
17 changes: 16 additions & 1 deletion tests/test_builders/test_build_latex.py
Original file line number Diff line number Diff line change
Expand Up @@ -2220,7 +2220,6 @@ def test_one_parameter_per_line(app):
app.build(force_all=True)
result = (app.outdir / 'projectnamenotset.tex').read_text(encoding='utf8')

# TODO: should these asserts check presence or absence of a final \sphinxparamcomma?
# signature of 23 characters is too short to trigger one-param-per-line mark-up
assert (
'\\pysiglinewithargsret\n'
Expand All @@ -2233,6 +2232,7 @@ def test_one_parameter_per_line(app):
'{\\sphinxbfcode{\\sphinxupquote{foo}}}\n'
'{\\sphinxoptional{\\sphinxparam{' in result
)
assert r'\sphinxparam{\DUrole{n}{f}}\sphinxparamcomma' in result

# generic_arg[T]
assert (
Expand Down Expand Up @@ -2289,6 +2289,21 @@ def test_one_parameter_per_line(app):
)


@pytest.mark.sphinx(
'latex',
testroot='domain-py-python_maximum_signature_line_length',
confoverrides={
'python_maximum_signature_line_length': 23,
'python_trailing_comma_in_multi_line_signatures': False,
},
)
def test_one_parameter_per_line_without_trailing_comma(app):
app.build(force_all=True)
result = (app.outdir / 'projectnamenotset.tex').read_text(encoding='utf8')

assert r'\sphinxparam{\DUrole{n}{f}}}}' in result


@pytest.mark.sphinx('latex', testroot='markup-rubric')
def test_latex_rubric(app):
app.build()
Expand Down
4 changes: 2 additions & 2 deletions tests/test_domains/test_domain_c.py
Original file line number Diff line number Diff line change
Expand Up @@ -1370,7 +1370,7 @@ def test_domain_c_c_maximum_signature_line_length_in_html(app):
<dd>\
<span class="n"><span class="pre">str</span></span>\
<span class="w"> </span>\
<span class="n"><span class="pre">name</span></span>,\
<span class="n"><span class="pre">name</span></span>\
</dd>
</dl>

Expand All @@ -1391,6 +1391,6 @@ def test_domain_c_c_maximum_signature_line_length_in_text(app):
content = (app.outdir / 'index.txt').read_text(encoding='utf8')
param_line_fmt = STDINDENT * ' ' + '{}\n'

expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name,'))
expected_parameter_list_hello = '(\n{})'.format(param_line_fmt.format('str name'))

assert expected_parameter_list_hello in content
Loading
Loading