Skip to content

Enable optional skipping of null bytes on dump#883

Merged
ohler55 merged 5 commits intoohler55:developfrom
altmetric:enable-null-byte-skipping
Jun 1, 2023
Merged

Enable optional skipping of null bytes on dump#883
ohler55 merged 5 commits intoohler55:developfrom
altmetric:enable-null-byte-skipping

Conversation

@ttr3dp
Copy link
Copy Markdown
Contributor

@ttr3dp ttr3dp commented May 31, 2023

Closes: #882

Why?

See #882

What's changed?

  • A :skip_null_byte boolean option is introduced. If set to true, null bytes will be omitted from strings when dumping.
  • Documentation is updated (docs/mode_table.html and Pages/Options.md)

Example

Oj.dump({ "key\x00" => "val\x00ue" }, :skip_null_byte => true)

# => "{\"key\":\"value\"}"

Comment thread ext/oj/dump.c Outdated
@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented May 31, 2023

You covered all the bases. Nice. Just a couple of comments on the change in dump.c.

@ttr3dp ttr3dp requested a review from ohler55 May 31, 2023 20:47
@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 1, 2023

Some formatting issues. If you want to fix them that would be great otherwise I'll merge and fix after merge.

I probably should have resisted the formatters. Nothing but a headache.

@ttr3dp
Copy link
Copy Markdown
Contributor Author

ttr3dp commented Jun 1, 2023

Some formatting issues. If you want to fix them that would be great otherwise I'll merge and fix after merge.

I probably should have resisted the formatters. Nothing but a headache.

I fixed 'em.
Although, I'm not sure what are you thoughts on moving dev dependencies from gemspec to Gemfile?
Let me know if this should be in .rubocop_todo.yml maybe? 🤔

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 1, 2023

Thank you. My opinion on formatters or linters is generally not good when writing my own project but when working with others it does help. Rubocop is annoying because it keeps changing the rules. gem spec vs Gemfile, I don't care as long as it doesn't keep changing.

I merge with Rubocop failures and fix after the merge. There is no reason for you to forced into that.

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 1, 2023

Thank you. My opinion on formatters or linters is generally not good when writing my own project but when working with others it does help. Rubocop is annoying because it keeps changing the rules. gem spec vs Gemfile, I don't care as long as it doesn't keep changing.

I'll merge with Rubocop failures and fix after the merge. There is no reason for you to forced into that. I am seeing other failures though that were not there on the last run.

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 1, 2023

Maybe revert the gemfile change and go back to just rubicon failing and the rest passing.

@ttr3dp
Copy link
Copy Markdown
Contributor Author

ttr3dp commented Jun 1, 2023

Maybe revert the gemfile change and go back to just rubicon failing and the rest passing.

Done!

@ohler55 ohler55 merged commit b50f308 into ohler55:develop Jun 1, 2023
@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 1, 2023

Thanks, I'll see what I can do about the Rubocop issue and release tonight.

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 2, 2023

It's going to take a bit more to get all the rubocop stuff addressed.

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 2, 2023

I'll move and rename the option as well. Sorry, I should have noticed it was not in the dump options. Renaming it to omit_null_byte for consistency. Would get it done along with the rubocop stuff tonight.

@ohler55
Copy link
Copy Markdown
Owner

ohler55 commented Jun 3, 2023

v3.15.0 released with :omit_null_byte option.

@ttr3dp
Copy link
Copy Markdown
Contributor Author

ttr3dp commented Jun 5, 2023

THANK YOU for literally everything. Time, patience, understanding, sharing the knowledge and so much more.

As for the changes...I agree, omit_null_byte is more consistent 👍

I'm truly glad I could contribute to such a useful project and learn a lot in the process.

Thanks again!

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.

Skipping null bytes on dump

2 participants