Export git_buf_text_is_binary and git_buf_text_contains_nul. #2475

Merged
merged 2 commits into from Jul 16, 2014

Projects

None yet

4 participants

@joshaber
Member

So that users don’t need to implement binary detection themselves.

@joshaber joshaber Export git_buf_text_is_binary and git_buf_text_contains_nul.
So that users don’t need to implement binary detection themselves.
df4cba0
@arrbee
Member
arrbee commented Jul 16, 2014

Hmm, I feel like if we're going to expose these functions that instead of adding another public header, it would be tempting to just add them as git_buffer_is_binary and git_buffer_contains_nul and either make 1-line wrappers with the new names or just rename the old functions. Of course, @vmg may disagree, but this doesn't feel like significant enough functionality the keep distinct from include/git2/buffer.h.

@joshaber
Member

That's cool too! I was looking at buf_text.h and it seemed like there was other stuff in there that we might want to export in the future. But I'm happy to keep it all in buffer.h if that's what you guys would prefer.

@linquize
Contributor

Have you notice the problem in #2436?

@vmg
Member
vmg commented Jul 16, 2014

Definitely agree with @arrbee let's put this in the buf namespace, at least with a thin wrapper.

@joshaber
Member

🆒

Moved it to buf.

@vmg vmg merged commit 091165c into master Jul 16, 2014

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details
@joshaber
Member

🤘

@joshaber joshaber deleted the expose-buffer-binary-detection branch Jul 16, 2014
@linquize
Contributor

Hi, does anyone notice the problem in #2436?
And you expose them before git_buf_text_is_binary() is corrected.

@joshaber
Member

Well, to be fair we expose both. Users can decide if they want the simpler NUL check or the heuristic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment