hyphen: Add min-spaces-after and check-scalars options

[sbaudoin]

This permits checking style when people want sequences to have a fixed number of spaces greater than 1 for example.
We may also use these options to identify typos where we expect everything starting with an hyphen to be a sequence.

[coveralls]

coverage: 99.401% (+0.005%) from 99.396%
when pulling 15dbaed on sbaudoin:hyphen-min-spaces
into 8713140 on adrienverge:master.

[adrienverge]

Hello and thanks for contributing!

The min-spaces-after change is legitimate, and could be merged soon. Please see my proposals in the comments below.

About the check-scalars option, I'm not sure I'm favorable to it. And checking every scalar for a starting - in a YAML file is probably very bad for performance. But maybe I don't understand its use and usefulness for yamllint. Can you please propose it in a different pull request or issue, with example use-cases?

(For info, the examples you give:

object: [elem1, ...]
-bar:

would not be valid YAML anyway if the scalar -bar: was turned into a list item - bar:.)

[adrienverge]

I prefer removing this, because otherwise it would make sense to perform the same check for min-spaces-after too (which isn't allowed to be zero), and probably in many other rules too.

[adrienverge]

# Disabled isn't noted for other rules with -1 as default value (braces, brackets...) For consistency, I propose not to add here neither.

Or even better: let's set min-spaces-after: 1 as the default (instead of -1), and as the disabled value, since having less than one space results in a syntax error anyway. It should add some clarity for users. What do you think?

Also can you place min-spaces-after option before max-spaces-after?

[adrienverge]

This is a bit irrelevant to the example, and only adds confusion in my opinion. Let's remove it?

[adrienverge]

Same here: this seems irrelevant and adds confusion: let's remove it.

[adrienverge]

1. Suggested change

   	* ``max-spaces-after`` defines the maximal number of spaces allowed after
   	hyphens.
   	hyphens. Set to a negative integer if you want to allow any number of
   	spaces.
   	* ``max-spaces-after`` defines the maximal number of spaces allowed after
   	hyphens. Set to ``-1`` if you want to allow any number of spaces.
   (just in case we want to reserve other magic values like -2 in the future)
2. Can you place max-spaces-after after min-spaces-after?

[adrienverge]

I propose to be a bit simpler:

Suggested change

	* ``min-spaces-after`` defines the minimal number of spaces expected after
	hyphens. Set to a negative integer if you want to allow any number of
	spaces. When set to a positive value, cannot be greater than
	``max-spaces-after``.
	* ``min-spaces-after`` defines the minimal number of spaces expected after
	hyphens. The default and minimum value is ``1``.

(the rest is obvious and will throw a configuration error if needed)

[adrienverge]

Doing the check if conf['min-spaces-after'] == 1 is useless (it cannot be lower than 1) and will degrade performance.

I propose:

Suggested change

	if conf['min-spaces-after'] > 0:
	if conf['min-spaces-after'] > 1:

[sbaudoin]

I'm going to push additional commits. For a cleaner history, will you do the rebase or should I do it on my side?

About the rational behind check-scalars, this came from the same code extract as the one you give:

foo:
     -broken

From a YAML point of view this is a valid scalar syntax but as we see that the scalar starts with an hyphen we may think this syntax is a typo. On the first sight we may think that hyphen-min-spaces will identify that but not as we are not considering a sequence because this is a scalar. So I added check-scalars to be able to check such situations. But I agree that this might lead to poor performances and may trigger a lot of false positive (that's explain in the doc).

From my point of view, hyphen-min-spaces would be incomplete without this additional option; we can go without it but we must be clear about such situations not being taken into account.

[adrienverge]

I'm going to push additional commits. For a cleaner history, will you do the rebase or should I do it on my side?

OK! The best would be to rebase and squash into one commit with a nice title, ready to be merged.

From my point of view, hyphen-min-spaces would be incomplete without this additional option; we can go without it but we must be clear about such situations not being taken into account.

Thanks for the explanation.
I'm +1 to be more clear about such situations not taken into account (and the reason is simple: they are valid YAML). The quoted-strings rule can also be enabled to avoid such problems.