Skip to content

Add options to support TextMarshaler and TextUnmarshaler#672

Merged
fxamacker merged 3 commits intofxamacker:masterfrom
benluddy:text-un-marshaler
Jun 16, 2025
Merged

Add options to support TextMarshaler and TextUnmarshaler#672
fxamacker merged 3 commits intofxamacker:masterfrom
benluddy:text-un-marshaler

Conversation

@benluddy
Copy link
Copy Markdown
Contributor

@benluddy benluddy commented Jun 11, 2025 •
edited
Loading

Description

This adds TextMarshaler and TextUnmarshaler options to EncOptions and DecOptions, respectively. They're essentially derivative of the existing options for BinaryMarshaler and BinaryUnmarshaler.

Enabling this improves drop-in compatibility with encoding/json, which similarly marshals non-json.Marshaler TextMarshalers to strings and unmarshalers non-json.Unmarshaler TextUnmarshalers from strings.

PR Was Proposed and Welcomed in Currently Open Issue

Checklist (for code PR only, ignore for docs PR)

  • Include unit tests that cover the new code
  • Pass all unit tests
  • Pass all lint checks in CI (goimports, gosec, staticcheck, etc.)
  • Sign each commit with your real name and email.
    Last line of each commit message should be in this format:
    Signed-off-by: Firstname Lastname firstname.lastname@example.com
  • Certify the Developer's Certificate of Origin 1.1
    (see next section).

Certify the Developer's Certificate of Origin 1.1

  • By marking this item as completed, I certify
    the Developer Certificate of Origin 1.1.
Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
660 York Street, Suite 102,
San Francisco, CA 94110 USA

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.

Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

@benluddy
Copy link
Copy Markdown
Contributor Author

benluddy commented Jun 11, 2025

Oops, missed some test coverage. I'll take care of that.

@benluddy benluddy force-pushed the text-un-marshaler branch 2 times, most recently from be1d3d4 to 86e564e Compare June 11, 2025 16:20
fxamacker
fxamacker reviewed Jun 14, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@fxamacker fxamacker left a comment

Choose a reason for hiding this comment

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

Thanks @benluddy for opening this PR! I left some very minor comments.

We already discussed adding this feature, so I opened issue #675 just to have an open issue to link to that can be closed by this PR.

@benluddy benluddy force-pushed the text-un-marshaler branch from 86e564e to 1c2d1c1 Compare June 16, 2025 13:39
benluddy added 3 commits June 16, 2025 09:43
@benluddy
Add option to marshal encoding.TextMarshalers to CBOR text strings.
7a9dbd0 
The default behavior (encoding.TextMarshaler not recognized) remains unchanged.

Signed-off-by: Ben Luddy <bluddy@redhat.com>
@benluddy
Add option to decode text strings into encoding.TextUnmarshalers.
bf9e7d0 
The default retains the existing behavior and ignores TextUnmarshaler implementations.

Signed-off-by: Ben Luddy <bluddy@redhat.com>
@benluddy
Replace empty encode mode test case names.
db9afc5
@benluddy benluddy force-pushed the text-un-marshaler branch from 1c2d1c1 to db9afc5 Compare June 16, 2025 13:44
@benluddy benluddy requested a review from fxamacker June 16, 2025 13:47
fxamacker
fxamacker approved these changes Jun 16, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@fxamacker fxamacker left a comment

Choose a reason for hiding this comment

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

Thank you! 👍

@fxamacker fxamacker merged commit a8dbe3e into fxamacker:master Jun 16, 2025
20 checks passed
@fxamacker fxamacker mentioned this pull request Jun 16, 2025
Closed
@fxamacker fxamacker added this to the v2.9.0 milestone Jul 5, 2025
@fxamacker fxamacker added the enhancement New feature or request label Jul 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@fxamacker fxamacker fxamacker approved these changes

Assignees

No one assigned

Labels

enhancement New feature or request

Projects

None yet

Milestone

v2.9.0

Development

Successfully merging this pull request may close these issues.

2 participants

@benluddy @fxamacker