-
Notifications
You must be signed in to change notification settings - Fork 281
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
hyphen: Add min-spaces-after and check-scalars options #606
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -19,7 +19,20 @@ | |||||||||||||
.. rubric:: Options | ||||||||||||||
|
||||||||||||||
* ``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. | ||||||||||||||
* ``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``. | ||||||||||||||
Comment on lines
+24
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I propose to be a bit simpler:
Suggested change
(the rest is obvious and will throw a configuration error if needed) |
||||||||||||||
* YAMLLint will consider ``-xx`` as a scalar. However you may consider | ||||||||||||||
that, in your context, such a syntax is a typo and is actually a sequence | ||||||||||||||
and as a consequence there should be a space after the hyphen. As this is | ||||||||||||||
not a standard behaviour, you explicitly need to enable this control by | ||||||||||||||
setting the option ``check-scalars`` to ``true``. **Use with caution** | ||||||||||||||
as all scalars will be checked and non-solvable false positive might be | ||||||||||||||
identified. Has no effect when set to ``true`` but ``min-spaces-after`` | ||||||||||||||
is disabled (< 0). | ||||||||||||||
|
||||||||||||||
.. rubric:: Default values (when enabled) | ||||||||||||||
|
||||||||||||||
|
@@ -28,6 +41,8 @@ | |||||||||||||
rules: | ||||||||||||||
hyphens: | ||||||||||||||
max-spaces-after: 1 | ||||||||||||||
min-spaces-after: -1 # Disabled | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Or even better: let's set Also can you place |
||||||||||||||
check-scalars: false | ||||||||||||||
|
||||||||||||||
.. rubric:: Examples | ||||||||||||||
|
||||||||||||||
|
@@ -72,24 +87,97 @@ | |||||||||||||
- key | ||||||||||||||
- key2 | ||||||||||||||
- key42 | ||||||||||||||
|
||||||||||||||
#. With ``hyphens: {min-spaces-after: 3}`` | ||||||||||||||
|
||||||||||||||
the following code snippet would **PASS**: | ||||||||||||||
:: | ||||||||||||||
|
||||||||||||||
list: | ||||||||||||||
- key | ||||||||||||||
- key2 | ||||||||||||||
- key42 | ||||||||||||||
-foo: # starter of a new sequence named "-foo"; | ||||||||||||||
# without the colon, a syntax error will be raised. | ||||||||||||||
Comment on lines
+100
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bit irrelevant to the example, and only adds confusion in my opinion. Let's remove it? |
||||||||||||||
|
||||||||||||||
the following code snippet would **FAIL**: | ||||||||||||||
:: | ||||||||||||||
|
||||||||||||||
- key | ||||||||||||||
- key2 | ||||||||||||||
- key42 | ||||||||||||||
|
||||||||||||||
#. With ``hyphens: {min-spaces-after: 3, check-scalars: true}`` | ||||||||||||||
|
||||||||||||||
the following code snippet would **PASS**: | ||||||||||||||
:: | ||||||||||||||
|
||||||||||||||
list: | ||||||||||||||
- key | ||||||||||||||
- key2 | ||||||||||||||
- key42 | ||||||||||||||
key: -value | ||||||||||||||
|
||||||||||||||
the following code snippets would **FAIL**: | ||||||||||||||
:: | ||||||||||||||
|
||||||||||||||
--- | ||||||||||||||
-item0 | ||||||||||||||
|
||||||||||||||
:: | ||||||||||||||
|
||||||||||||||
sequence: | ||||||||||||||
-key # Mind the spaces before the hyphen to enforce | ||||||||||||||
# the sequence and avoid a syntax error | ||||||||||||||
Comment on lines
+127
to
+131
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here: this seems irrelevant and adds confusion: let's remove it. |
||||||||||||||
""" | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
import yaml | ||||||||||||||
|
||||||||||||||
from yamllint.linter import LintProblem | ||||||||||||||
from yamllint.rules.common import spaces_after | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
ID = 'hyphens' | ||||||||||||||
TYPE = 'token' | ||||||||||||||
CONF = {'max-spaces-after': int} | ||||||||||||||
DEFAULT = {'max-spaces-after': 1} | ||||||||||||||
CONF = {'max-spaces-after': int, | ||||||||||||||
'min-spaces-after': int, | ||||||||||||||
'check-scalars': bool} | ||||||||||||||
DEFAULT = {'max-spaces-after': 1, | ||||||||||||||
'min-spaces-after': -1, | ||||||||||||||
'check-scalars': False} | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
def VALIDATE(conf): | ||||||||||||||
if conf['max-spaces-after'] == 0: | ||||||||||||||
return '"max-spaces-after" cannot be set to 0' | ||||||||||||||
Comment on lines
+152
to
+153
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I prefer removing this, because otherwise it would make sense to perform the same check for |
||||||||||||||
if (conf['min-spaces-after'] > 0 and | ||||||||||||||
conf['min-spaces-after'] > conf['max-spaces-after']): | ||||||||||||||
return '"min-spaces-after" cannot be greater than "max-spaces-after"' | ||||||||||||||
|
||||||||||||||
|
||||||||||||||
def check(conf, token, prev, next, nextnext, context): | ||||||||||||||
if isinstance(token, yaml.BlockEntryToken): | ||||||||||||||
problem = spaces_after(token, prev, next, | ||||||||||||||
max=conf['max-spaces-after'], | ||||||||||||||
max_desc='too many spaces after hyphen') | ||||||||||||||
if problem is not None: | ||||||||||||||
yield problem | ||||||||||||||
if conf['max-spaces-after'] > 0: | ||||||||||||||
problem = spaces_after(token, prev, next, | ||||||||||||||
max=conf['max-spaces-after'], | ||||||||||||||
max_desc='too many spaces after hyphen') | ||||||||||||||
if problem is not None: | ||||||||||||||
yield problem | ||||||||||||||
|
||||||||||||||
if conf['min-spaces-after'] > 0: | ||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doing the check if I propose:
Suggested change
|
||||||||||||||
problem = spaces_after(token, prev, next, | ||||||||||||||
min=conf['min-spaces-after'], | ||||||||||||||
min_desc='too few spaces after hyphen') | ||||||||||||||
if problem is not None: | ||||||||||||||
yield problem | ||||||||||||||
|
||||||||||||||
if (conf['check-scalars'] and conf['min-spaces-after'] > 0 | ||||||||||||||
and isinstance(token, yaml.ScalarToken)): | ||||||||||||||
# Token identified as a scalar so there is no space after the | ||||||||||||||
# hyphen: no need to count | ||||||||||||||
if token.value.startswith('-'): | ||||||||||||||
yield LintProblem( | ||||||||||||||
token.start_mark.line + 1, | ||||||||||||||
token.start_mark.column + 1, | ||||||||||||||
'too few spaces after hyphen') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-2
in the future)max-spaces-after
aftermin-spaces-after
?