Avoid invalid combination of force-sort-within-types and lines-between-types
#9041
Conversation
|
| @@ -2109,6 +2109,21 @@ impl IsortOptions { | |||
| warn_user_once!("`sections` is ignored when `no-sections` is set to `true`"); | |||
| } | |||
|
|
|||
| // Verify that if `force_sort_within_sections` is `True`, then `lines_between_types` is set to `0`. | |||
| let force_sort_within_sections = self.force_sort_within_sections.unwrap_or_default(); | |||
There was a problem hiding this comment.
Shouldn't we warn in the settings instead? Invalid configurations are otherwise still possible when using extend.
It may also be worth to aborting in that case to prevent that we change a large set of files because we ignored the setting.
There was a problem hiding this comment.
We already ignore this in the underlying implementation, and warning here is consistent with the other validation in this method. If we want to warn on Settings, I think that would be a separate change.
There was a problem hiding this comment.
Interesting, the formatter validations are done inside of settings to avoid these inconsistencies.
I think validating in settings only is sufficient. Only validating in options has the downside that we might ignore the option without a warning if one isort options comes from the extended configuration
There was a problem hiding this comment.
Interesting, the formatter validations are done inside of settings to avoid these inconsistencies.
I think validating in settings only is sufficient. Only validating in options has the downside that we might ignore the option without a warning if one isort options comes from the extended configuration
charliermarsh
force-pushed
the
charlie/force-sort
branch
from
d05fa69 to
b03373d
Compare
Closes #8792.