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

Add line_wrap to encoding config #49

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Add line_wrap to encoding config #49

merged 3 commits into from
Jul 25, 2023

Conversation

accidentaldevelopment
Copy link
Contributor

This PR adds a line_wrap field to the EncodeConfig struct that determines the line length for the encoding.

Unfortunately this is a breaking change. To avoid it being a breaking change in the future, I made the fields private and added chain-able setter methods. Adding fields going forward would not be a breaking change, just a new feature.

After writing this, I came up with another idea that would avoid a breaking change. I could create a struct Encoder that essentially has the same state as EncodeConfig but private. Encoder::encode(&self, &pem) could handle encoding a PEM with the specified config without requiring any changes to the existing encode function or EncodeConfig struct.

I can put up a separate MR with that change if that is better, or if you'd like to compare.

Resolves #48

@jcreekmore
Copy link
Owner

What you did is just fine. That is the approach I was going to take.

It looks like a sub-sub-sub dependency for testing is breaking that MSRV test. I am trying to figure out if I can make that failure go away because I don't think I even need the crate pulled in that moved forward to 1.63.

@jcreekmore
Copy link
Owner

OK, I fixed up master. Can you rebase this on top of master and also add a CHANGELOG entry? Maybe something like "make EncodeConfig struct extendable and add a line_wrap config option"?

@accidentaldevelopment
Copy link
Contributor Author

Done and done!

@accidentaldevelopment
Copy link
Contributor Author

Doc tests, always fun :). Fix incoming.

@jcreekmore jcreekmore merged commit bc5d3bb into jcreekmore:master Jul 25, 2023
1 check passed
@jcreekmore
Copy link
Owner

I went ahead and published 3.0.0 with this change.

@accidentaldevelopment
Copy link
Contributor Author

That's amazing, thank you the quick turnaround!

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.

Configurable line length for encoding
2 participants