Skip to content

Conversation

@ghost
Copy link

@ghost ghost commented May 1, 2025

In the current version, the error message states TypeError: 'NoneType' object is not iterable when the 'rules' in the config file is not a dict (gh-274.) It would be helpful to improve the error message.

Additionally, it is for consistency, as other elements(ignore, locale, etc.) have similar error checks.

@coveralls
Copy link

Coverage Status

coverage: 99.815%. remained the same
when pulling a941fb4 on koyuki7w:patch-1
into 2d10aaa on adrienverge:master.

Copy link
Owner

@adrienverge adrienverge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello Yuki,

Thanks for this addition, this is a good idea.

I wonder whether "dict" is the best user-facing word (in "rules should be a dict"), because "dict" seems a Python-specific word to me. But anyway, we can always change it later if needed.
EDIT: We already use "dict" a few lines above (in "invalid config: not a dict") so I guess it's OK.

@adrienverge adrienverge merged commit b6fe397 into adrienverge:master May 4, 2025
7 checks passed
@ghost ghost deleted the patch-1 branch May 4, 2025 12:45
@ghost
Copy link
Author

ghost commented May 5, 2025

According to the YAML specification, "mapping" seems to be a more appropriate term. The current yamllint documentation uses "mapping" and does not mention "dict."

Perhaps all three instances of "dict" in the error message should be changed to "mapping."

raise YamlLintConfigError('invalid config: not a dict')
raise YamlLintConfigError('invalid config: rules should be a dict')

yamllint/yamllint/config.py

Lines 237 to 238 in 79a6b2b

f'invalid config: rule "{rule.ID}": should be either "enable", '
f'"disable" or a dict')

adrienverge added a commit that referenced this pull request May 5, 2025
Previously, these errors used the word "dict" because it's the common
term in the Python world. But the YAML term is rather "mapping". The
present commit fixes this wording.

This originates from this discussion [^2].

[^1]: https://yaml.org/spec/1.2.2/#3211-nodes
[^2]: #747 (review)
adrienverge added a commit that referenced this pull request May 5, 2025
Previously, these errors used the word "dict" because it's the common
term in the Python world. But the YAML term is rather "mapping". The
present commit fixes this wording.

This originates from this discussion [^2].

[^1]: https://yaml.org/spec/1.2.2/#3211-nodes
[^2]: #747 (review)
@adrienverge
Copy link
Owner

Thanks for your input @koyuki7w. I agree and I created #751 to address this.

adrienverge added a commit that referenced this pull request May 5, 2025
Previously, these errors used the word "dict" because it's the common
term in the Python world. But the YAML term is rather "mapping". The
present commit fixes this wording.

This originates from this discussion [^2].

[^1]: https://yaml.org/spec/1.2.2/#3211-nodes
[^2]: #747 (review)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants