-
Notifications
You must be signed in to change notification settings - Fork 144
[BUGFIX] Drop test that passes in the wrong type #866
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
Conversation
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.
OutputFormat::spaceAfterListArgumentSeparator
is expected to be a string, not an array.
I think it is expected to be a string or an array. If an array, the keys are list separators and the values are the space string to insert after them, the the first item in the array (also) serving as the default space separator. See OutputFormat::createPretty()
for examples and OutputFormmatter::space()
for implementation.
6e88a94
to
4a959bc
Compare
Yes, that's how it currently works (even if it violates the type annotation). This behavior is not documented, and I'd like to get rid of it. |
It's documented by this:
It's also part of the public API. So if we remove it we'll first need to mark it as deprecated. |
`OutputFormat::spaceAfterListArgumentSeparator` is expected to be a string, not an array. This was discovered by adding dedicated, strictly typed property accessors for `OutputFormat`.
4a959bc
to
7b11d2a
Compare
If we do want to remove the functionality, we'd first need to create the dedicated (non-majic) methods (which @oliverklee I see you're on a path towards, e.g. with #867), whilst retaining current functionality, and correcting the type definitions to include Then it would be possibke to deprecate the array parameter option to have different spacing depending on list separator, with e.g. Having done the first part, it would be extra effort to remove the feature. I think we may as well retain it. @oliverklee, @sabberworm, WDYT? |
`SpaceBeforeListArgumentSeparators` and `SpaceAfterListArgumentSeparators` are added to allow for different spacing for different separators. `SpaceBeforeListArgumentSeparator` and `SpaceAfterListArgumentSeparator` will specify the default spacing. Setting these as an array is deprecated. Resolves #866, #876, #878.
OutputFormat::spaceAfterListArgumentSeparator
is expected to be a string, not an array.This was discovered by adding dedicated, strictly typed property accessors for
OutputFormat
.