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

Allow multiple parameters in lines of long parameter lists #11423

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 5 additions & 1 deletion sphinx/addnodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,11 @@ def astext(self):


class desc_parameter(nodes.Part, nodes.Inline, nodes.FixedTextElement):
"""Node for a single parameter."""
"""Node for a single parameter.

If the parent `desc_parameterlist` node has ``multi_line_parameter_list = True``,
set ``on_new_line = True`` to display on a new line.
"""


class desc_optional(nodes.Part, nodes.Inline, nodes.FixedTextElement):
Expand Down
44 changes: 39 additions & 5 deletions sphinx/domains/python.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,13 @@ def _parse_arglist(
params['multi_line_parameter_list'] = multi_line_parameter_list
sig = signature_from_str('(%s)' % arglist)
last_kind = None

if multi_line_parameter_list and env is not None:
max_param_line_length = env.config.python_maximum_signature_line_length
single_param_per_line = env.config.python_single_param_per_line_in_multiline_signatures
line_length = 0
is_first_param = True

for param in sig.parameters.values():
if param.kind != param.POSITIONAL_ONLY and last_kind == param.POSITIONAL_ONLY:
# PEP-570: Separator for Positional Only Parameter: /
Expand Down Expand Up @@ -300,6 +307,13 @@ def _parse_arglist(
node += nodes.inline('', param.default, classes=['default_value'],
support_smartquotes=False)

if multi_line_parameter_list: # TODO: for every params +=... -> define func?
param_str_length = len(node.astext()) + 2
line_length += param_str_length
if single_param_per_line or is_first_param or line_length > max_param_line_length:
is_first_param = False
node['on_new_line'] = True
line_length = param_str_length
params += node
last_kind = param.kind

Expand All @@ -311,7 +325,10 @@ def _parse_arglist(


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

Expand All @@ -321,10 +338,17 @@ def _pseudo_parse_arglist(
"""
paramlist = addnodes.desc_parameterlist()
paramlist['multi_line_parameter_list'] = multi_line_parameter_list
if multi_line_parameter_list and env is not None:
max_param_line_length = env.config.python_maximum_signature_line_length
single_param_per_line = env.config.python_single_param_per_line_in_multiline_signatures
line_length = 0
is_first_param = True

stack: list[Element] = [paramlist]
try:
for argument in arglist.split(','):
argument = argument.strip()
param_str_length = len(argument) + 2 # TODO: + closing optionals?
ends_open = ends_close = 0
while argument.startswith('['):
stack.append(addnodes.desc_optional())
Expand All @@ -340,8 +364,17 @@ def _pseudo_parse_arglist(
ends_open += 1
argument = argument[:-1].strip()
if argument:
stack[-1] += addnodes.desc_parameter(
'', '', addnodes.desc_sig_name(argument, argument))
node = addnodes.desc_parameter(
'', '', addnodes.desc_sig_name(argument, argument),
)
if multi_line_parameter_list:
line_length += param_str_length
goes_over_max_line_length = line_length > max_param_line_length
if single_param_per_line or is_first_param or goes_over_max_line_length:
is_first_param = False
node['on_new_line'] = True
line_length = param_str_length
stack[-1] += node
while ends_open:
stack.append(addnodes.desc_optional())
stack[-2] += stack[-1]
Expand Down Expand Up @@ -576,11 +609,11 @@ def handle_signature(self, sig: str, signode: desc_signature) -> tuple[str, str]
except SyntaxError:
# fallback to parse arglist original parser.
# it supports to represent optional arguments (ex. "func(foo [, bar])")
_pseudo_parse_arglist(signode, arglist, multi_line_parameter_list)
_pseudo_parse_arglist(signode, arglist, self.env, multi_line_parameter_list)
except NotImplementedError as exc:
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, self.env, multi_line_parameter_list)
else:
if self.needs_arglist():
# for callables, add an empty parameter list
Expand Down Expand Up @@ -1520,6 +1553,7 @@ def setup(app: Sphinx) -> dict[str, Any]:
app.add_config_value('python_use_unqualified_type_names', False, 'env')
app.add_config_value('python_maximum_signature_line_length', None, 'env',
types={int, None})
app.add_config_value('python_single_param_per_line_in_multiline_signatures', True, 'env')
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
22 changes: 11 additions & 11 deletions sphinx/writers/html5.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ def visit_desc_parameterlist(self, node: Element) -> None:
if self.multi_line_parameter_list:
self.body.append('\n\n')
self.body.append(self.starttag(node, 'dl'))
self.param_separator = self.param_separator.rstrip()

def depart_desc_parameterlist(self, node: Element) -> None:
if node.get('multi_line_parameter_list'):
Expand All @@ -179,12 +178,14 @@ def depart_desc_parameterlist(self, node: Element) -> None:
# foo([a, ]b, c[, d])
#
def visit_desc_parameter(self, node: Element) -> None:
on_separate_line = self.multi_line_parameter_list
on_separate_line = node.get('on_new_line')
if on_separate_line and not (self.is_first_param and self.optional_param_level > 0):
if not self.is_first_param:
self.body.append('</dd>\n')
self.body.append(self.starttag(node, 'dd', ''))
if self.is_first_param:
self.is_first_param = False
elif not on_separate_line and not self.required_params_left:
elif not self.multi_line_parameter_list and not self.required_params_left:
self.body.append(self.param_separator)
if self.optional_param_level == 0:
self.required_params_left -= 1
Expand All @@ -206,7 +207,8 @@ def depart_desc_parameter(self, node: Element) -> None:
opt_param_left_at_level = self.params_left_at_level > 0
if opt_param_left_at_level or is_required and (is_last_group or next_is_required):
self.body.append(self.param_separator)
self.body.append('</dd>\n')
if not opt_param_left_at_level and is_last_group:
self.body.append('</dd>\n')

elif self.required_params_left:
self.body.append(self.param_separator)
Expand All @@ -225,17 +227,14 @@ def visit_desc_optional(self, node: Element) -> None:
self.body.append(self.starttag(node, 'dd', ''))
self.body.append('<span class="optional">[</span>')
# Else, if there remains at least one required parameter, append the
# parameter separator, open a new bracket, and end the line.
# parameter separator and open a new bracket.
elif self.required_params_left:
self.body.append(self.param_separator)
self.body.append('<span class="optional">[</span>')
self.body.append('</dd>\n')
# Else, open a new bracket, append the parameter separator,
# and end the line.
# Else, open a new bracket and append the parameter separator.
else:
self.body.append('<span class="optional">[</span>')
self.body.append(self.param_separator)
self.body.append('</dd>\n')
else:
self.body.append('<span class="optional">[</span>')

Expand All @@ -248,8 +247,9 @@ def depart_desc_optional(self, node: Element) -> None:
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:
# optional parameter group and there is no group left.
is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
if self.optional_param_level == 0 and is_last_group:
self.body.append('</dd>\n')
else:
self.body.append('<span class="optional">]</span>')
Expand Down
27 changes: 13 additions & 14 deletions sphinx/writers/text.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,19 +605,19 @@ def visit_desc_parameterlist(self, node: Element) -> None:
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)
if self.multi_line_parameter_list:
self.param_separator = self.param_separator.rstrip()

def depart_desc_parameterlist(self, node: Element) -> None:
self.add_text(')')

def visit_desc_parameter(self, node: Element) -> None:
on_separate_line = self.multi_line_parameter_list
on_separate_line = node.get('on_new_line')
if on_separate_line and not (self.is_first_param and self.optional_param_level > 0):
if not self.is_first_param:
self.end_state(wrap=False, end=None)
self.new_state()
if self.is_first_param:
self.is_first_param = False
elif not on_separate_line and not self.required_params_left:
elif not self.multi_line_parameter_list and not self.required_params_left:
self.add_text(self.param_separator)
if self.optional_param_level == 0:
self.required_params_left -= 1
Expand All @@ -627,7 +627,7 @@ def visit_desc_parameter(self, node: Element) -> None:
self.add_text(node.astext())

is_required = self.list_is_required_param[self.param_group_index]
if on_separate_line:
if self.multi_line_parameter_list:
is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
next_is_required = (
not is_last_group
Expand All @@ -636,7 +636,8 @@ def visit_desc_parameter(self, node: Element) -> None:
opt_param_left_at_level = self.params_left_at_level > 0
if opt_param_left_at_level or is_required and (is_last_group or next_is_required):
self.add_text(self.param_separator)
self.end_state(wrap=False, end=None)
if not opt_param_left_at_level and is_last_group:
self.end_state(wrap=False, end=None)

elif self.required_params_left:
self.add_text(self.param_separator)
Expand All @@ -656,17 +657,14 @@ def visit_desc_optional(self, node: Element) -> None:
self.new_state()
self.add_text('[')
# Else, if there remains at least one required parameter, append the
# parameter separator, open a new bracket, and end the line.
# parameter separator and open a new bracket.
elif self.required_params_left:
self.add_text(self.param_separator)
self.add_text('[')
self.end_state(wrap=False, end=None)
# Else, open a new bracket, append the parameter separator, and end the
# line.
# Else, open a new bracket and append the parameter separator.
else:
self.add_text('[')
self.add_text(self.param_separator)
self.end_state(wrap=False, end=None)
else:
self.add_text('[')

Expand All @@ -678,9 +676,10 @@ def depart_desc_optional(self, node: Element) -> None:
if self.optional_param_level == self.max_optional_param_level - 1:
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:
# End the line if we have just closed the last bracket of this
# optional parameter group and there is no group left.
is_last_group = self.param_group_index + 1 == len(self.list_is_required_param)
if self.optional_param_level == 0 and is_last_group:
self.end_state(wrap=False, end=None)

else:
Expand Down