Let row_sep take a "policy" that decides for which definition lists a row separator must be used

[janluke]

Problems with row_sep

1. It affects the "Commands" section, which generally doesn't need the extra empty line / separator.

2. It affects all definition lists indiscriminately, including those that don't benefit from the extra empty line between definitions. Some people may like this consistency. Others, including me, may want to avoid the extra spacing when unneded.

3. Probably nobody is going to use a row separator other than '\n' (an extra new line), and probably shouldn't. Horizontal lines obtained by repeating one of the unicode "Box drawing" characters like '─' seem too much for separating definitions. This is not a real issue, just something I want to keep in mind: I'm open to "sacrifice" this rather useless flexibility.

Chosen solution: RowSepPolicy

Allow to pass a policy as row_sep:

class RowSepPolicy(metaclass=abc.ABCMeta):
    @abc.abstractmethod
    def __call__(  # noqa E704
        self, rows: Sequence[Sequence[str]],
        col_widths: Sequence[int],
        col_spacing: int,
    ) -> Optional[str]:
        """Decides which row separator to use (eventually none) in the given
        definition list."""

In practice, the row separator should be the same for all definition lists that satisfy a given condition (see RowSepIf below). So why this class is designed as it is? Mainly for one reason: it leaves open the door to policies that can decide a row separator for each individual row (feature I'm personally against to for now), without breaking changes. This would allow to implement the old Click 7.2 behavior, which inserted an empty line only after options with a long help (which I don't like). Adding this feature would be possible without breaking changes, by extending the return type to Union[None, str, Sequence[str]].

The concrete implementation that the user will actually use is much more pragmatic:

RowSepIf(condition: RowSepCondition, sep: Union[str, SepGenerator] = '\n')

where:

• RowSepCondition is a function (rows, col_widths, col_spacing) -> bool
• SepGenerator is a function (width: int) -> str that generates a separator given a line width.

Cloup will provide:

• a function multiline_rows_are_at_least(count_or_percentage) that returns a RowSepCondition

• an Hline(pattern: str) class that implements SepGenerator and has different static members for different kinds of line: Hline.solid, Hline.dashed, Hline.densely_dashed, Hline.dotted

So, in the end I decided to allow separators other than '\n' and Hline makes it trivial to use one. Plus, it may be useful in other parts of the program (e.g. print(Hline.solid(width), end='')).

For consistency, row_sep will also accept a SepGenerator (#39). So after this is implemented, row_sep will accept either:

• a string (e.g. '\n')
• a SepGenerator (e.g. Hline.dotted)
• a RowSepPolicy (e.g. RowSepIf(multiline_rows_are_at_least(2))).

Limitations

• A policy can't take decisions based on all sections. This would require serious refactoring.
• Intentionally, a policy can't decide a different row_sep for each definition. Nonetheless, the interface was designed to be flexible enough for extensions in this regard without breaking changes.

Evaluated solutions

The quick and easy one

I could just add row_sep to write_dl() so that the caller, e.g. format_commands, can set it to '' if opportune. This is also enough to allow format_options for eventually forcing row_sep to '' only for definition lists that don't need the extra spacing. Nonetheless this would either be hard-coded behaviour (difficult to override) or would require other formatting parameters in mixins, which I want to avoid.

Similar to chosen one, but allowing only for empty lines, not arbitrary strings

This would have required to replace row_sep with some other parameter.