Skip to content
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

Adjust single-line formatting behavior #26

Merged
merged 1 commit into from
Apr 12, 2024
Merged

Adjust single-line formatting behavior #26

merged 1 commit into from
Apr 12, 2024

Conversation

dsnet
Copy link
Collaborator

@dsnet dsnet commented Apr 12, 2024

Changes made:

  • Further separate the behavior of SpaceAfterColon and SpaceAfterComma from Multiline. That is, use of Multiline does not affect the behavior of SpaceAfterColon or SpaceAfterComma unless it has not been set. For example, specifying (SpaceAfterColon(false), Multiline(true)) should avoid the space after the colon even if we expect this combination of options to be seldom used.

  • Exclude whitespace formatting flags from DefaultOptionsV1 since v1 has API support for both single-line and multi-line output, so setting (or clearing) them cannot be classified as v1 behavior. Similarly, exclude whitespace formatting flags from DefaultOptionsV2.

  • Simplify the TestMarshal cases.

  • Add explicit TestEncoder cases to exercise all Encoder.WriteValue code paths.

Changes made:

* Further separate the behavior of SpaceAfterColon and SpaceAfterComma
  from Multiline. That is, use of Multiline does not affect the behavior
  of SpaceAfterColon or SpaceAfterComma unless it has not been set.
  For example, specifying (SpaceAfterColon(false), Multiline(true))
  should avoid the space after the colon even if we expect this
  combination of options to be seldom used.

* Exclude whitespace formatting flags from DefaultOptionsV1
  since v1 has API support for both single-line and multi-line output,
  so setting (or clearing) them cannot be classified as v1 behavior.
  Similarly, exclude whitespace formatting flags from DefaultOptionsV2.

* Simplify the TestMarshal cases.

* Add explicit TestEncoder cases to exercise all
  Encoder.WriteValue code paths.
@dsnet dsnet requested review from mvdan and johanbrandhorst April 12, 2024 01:01
@dsnet
Copy link
Collaborator Author

dsnet commented Apr 12, 2024

\cc @veqryn

@mvdan
Copy link
Collaborator

mvdan commented Apr 12, 2024

Thanks, done.

@mvdan mvdan merged commit 37be135 into master Apr 12, 2024
8 checks passed
@mvdan
Copy link
Collaborator

mvdan commented Apr 12, 2024

Err, I meant to merge #25, but this one also LGTM, so hopefully that's not a problem 😅

@dsnet
Copy link
Collaborator Author

dsnet commented Apr 12, 2024

No worries, it was done.

@dsnet dsnet deleted the single-line-flags branch April 12, 2024 20:21
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