Skip to content

Add binding for git_message_trailers #749

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.

Sign up for GitHub

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

Merged
alexcrichton merged 20 commits into from
Oct 18, 2021
Merged

Add binding for git_message_trailers #749

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
15a7d4e
Add binding for `git_message_trailers`
LemmingAvalanche Sep 27, 2021
5575c3d
`cargo fmt`
LemmingAvalanche Sep 27, 2021
33ebdcd
Fix pointer mismatch error
LemmingAvalanche Oct 11, 2021
f6fd4b7
Remove a lot of unnecessary lifetimes
LemmingAvalanche Oct 12, 2021
b0015cc
Remove another unnecessary lifetime
LemmingAvalanche Oct 12, 2021
e94008a
Use `Range<usize>` instead of `usize`
LemmingAvalanche Oct 12, 2021
e830f7d
`cargo fmt`
LemmingAvalanche Oct 12, 2021
55ea909
Inline one-off struct
LemmingAvalanche Oct 12, 2021
e7b0306
Implement more iterators
LemmingAvalanche Oct 13, 2021
cd4c2db
Undo accidental and unrelated edit
LemmingAvalanche Oct 14, 2021
e450945
Less explicit lifetimes
LemmingAvalanche Oct 15, 2021
a0bd026
Don’t need `std::marker` any more
LemmingAvalanche Oct 15, 2021
34fa7b5
Correct `len(…)`
LemmingAvalanche Oct 15, 2021
023fb92
Remove unnecessary annotation
LemmingAvalanche Oct 15, 2021
16321f3
Implement `size_hint()`
LemmingAvalanche Oct 15, 2021
70b7ebd
Split into “bytes” and “string” iterators
LemmingAvalanche Oct 15, 2021
ed11c76
Remove more lifetimes
LemmingAvalanche Oct 16, 2021
958e378
Docs
LemmingAvalanche Oct 16, 2021
2508506
`cargo fmt`
LemmingAvalanche Oct 16, 2021
f35b68f
Undo accidental and unrelated edit
LemmingAvalanche Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Inline one-off struct 
Suggested-by: Alex Crichton <[email protected]>
See: #749 (comment)
  • Loading branch information
@LemmingAvalanche
LemmingAvalanche committed Oct 12, 2021
commit 55ea9092cd08dd6bd9e7b9a2246d5b05935672e2
35 changes: 13 additions & 22 deletions src/message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,22 +88,6 @@ impl Binding for MessageTrailers {
}
}

struct Trailer<'pair> {
key: *const c_char,
value: *const c_char,
_marker: marker::PhantomData<&'pair c_char>,
}

impl<'pair> Trailer<'pair> {
fn to_str_tuple(self) -> (&'pair str, &'pair str) {
unsafe {
let key = CStr::from_ptr(self.key).to_str().unwrap();
let value = CStr::from_ptr(self.value).to_str().unwrap();
(key, value)
}
}
}

/// A borrowed iterator.
pub struct MessageTrailersIterator<'a> {
trailers: &'a MessageTrailers,
Expand All @@ -116,16 +100,23 @@ impl<'pair> Iterator for MessageTrailersIterator<'pair> {
fn next(&mut self) -> Option<Self::Item> {
self.range.next().map(|index| unsafe {
let addr = self.trailers.raw.trailers.wrapping_add(index);
Trailer {
key: (*addr).key,
value: (*addr).value,
_marker: marker::PhantomData,
}
.to_str_tuple()
to_str_tuple((*addr).key, (*addr).value, marker::PhantomData)
})
}
Copy link
Member

@alexcrichton alexcrichton Oct 15, 2021

Choose a reason for hiding this comment

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

Could you add size_hint here too?

Copy link
Contributor Author

@LemmingAvalanche LemmingAvalanche Oct 16, 2021

Choose a reason for hiding this comment

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

Ah okay I think I understand now. I confused myself thought that size_hint() was part of ExactSizeIterator.

See rev 16321f3

}

fn to_str_tuple<'pair>(
key: *const c_char,
value: *const c_char,
_marker: marker::PhantomData<&'pair c_char>,
) -> (&'pair str, &'pair str) {
unsafe {
let k = CStr::from_ptr(key).to_str().unwrap();
let v = CStr::from_ptr(value).to_str().unwrap();
(k, v)
}
}
Copy link
Member

@alexcrichton alexcrichton Oct 11, 2021

Choose a reason for hiding this comment

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

Could you add implementations for size_hint and ExactSizeIterator here as well? (which would also necessitate DoubleEndedIterator, and the implementations should be relatively easy if the counter field is changed to a Range)

Copy link
Contributor Author

@LemmingAvalanche LemmingAvalanche Oct 14, 2021

Choose a reason for hiding this comment

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

I implemented len() for ExactSizeIterator. The doc says that size_hint() can be derived from it. I couldn’t implement size_hint() myself.

The doc says that len() has a default implementation, but I guess just returning the value directly wouldn’t be less efficient than whatever the default impl. is.

See rev. e7b0306


/// Get the trailers for the given message.
pub fn message_trailers<'pair, S: IntoCString>(message: S) -> Result<MessageTrailers, Error> {
_message_trailers(message.into_c_string()?)
Expand Down
2 changes: 1 addition & 1 deletion src/util.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ pub fn c_cmp_to_ordering(cmp: c_int) -> Ordering {
pub fn path_to_repo_path(path: &Path) -> Result<CString, Error> {
macro_rules! err {
($msg:literal, $path:expr) => {
return Err(Error::from_str(&format!($msg, $path.display())))
return Err(Error::from_str(&format!($msg, $path.display())));
};
}
match path.components().next() {
Expand Down