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.

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

Add bindings for git_index_find_prefix #903

Merged
merged 4 commits into from
Feb 27, 2023
Merged

Add bindings for git_index_find_prefix #903

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f8ac2f7
implement find_prefix for index
zeerooth Dec 12, 2022
c1e4ae1
add git_index_find_prefix to raw
zeerooth Dec 12, 2022
4a67c0b
use generic IntoCString instead of only path as a prefix arg for find…
zeerooth Dec 13, 2022
d73c3f8
remove GIT_ENOTFOUND check and instead return it as an Err
zeerooth Jan 9, 2023
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
5 changes: 5 additions & 0 deletions libgit2-sys/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2953,6 +2953,11 @@ extern "C" {
pub fn git_index_entrycount(entry: *const git_index) -> size_t;
pub fn git_index_find(at_pos: *mut size_t, index: *mut git_index, path: *const c_char)
-> c_int;
pub fn git_index_find_prefix(
at_pos: *mut size_t,
index: *mut git_index,
prefix: *const c_char,
) -> c_int;
pub fn git_index_free(index: *mut git_index);
pub fn git_index_get_byindex(index: *mut git_index, n: size_t) -> *const git_index_entry;
pub fn git_index_get_bypath(
Expand Down
38 changes: 38 additions & 0 deletions src/index.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,6 +596,26 @@ impl Index {
Ok(Binding::from_raw(&raw as *const _))
}
}

/// Find the first position of any entries matching a prefix.
///
/// To find the first position of a path inside a given folder, suffix the prefix with a '/'.
pub fn find_prefix<T: IntoCString>(&self, prefix: T) -> Result<Option<usize>, Error> {
unsafe {
let mut at_pos: size_t = 0;
let entry_path = prefix.into_c_string()?;
let result = call!(raw::git_index_find_prefix(
&mut at_pos,
self.raw,
entry_path
));
if result == 0 {
Ok(Some(at_pos))
} else {
Ok(None)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't ever seem to return an error. Should this perhaps be:

Suggested change
if result == 0 {
Ok(Some(at_pos))
} else {
Ok(None)
}
if result == 0 {
Ok(Some(at_pos))
} else if result == raw::GIT_ENOTFOUND {
Ok(None)
} else {
Err(Error::last_error(result).unwrap())
}

I understand git_index_find_prefix currently never returns any other error, but I'm not very comfortable assuming that will be the case forever.

Also, I'm a little uncomfortable doing the ENOTFOUND check. I think it makes for a more natural Rust interface, but I don't see any other methods in the library doing that (they all just return NotFound). What were your thoughts on this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I imagined that returning None in situations when the prefix hasn't been found would be more rusty, but after reviewing some other similar functions, it seems like NotFound error is never handled this way, and as there's no point in going against the design of the rest of the library I just decided to leave it inside the Error, as you suggested.

I hope it's okay now

}
}
}

impl Binding for Index {
Expand Down Expand Up @@ -857,6 +877,24 @@ mod tests {
assert_eq!(e.path.len(), 6);
}

#[test]
fn add_then_find() {
let mut index = Index::new().unwrap();
let mut e = entry();
e.path = b"foo/bar".to_vec();
index.add(&e).unwrap();
let mut e = entry();
e.path = b"foo2/bar".to_vec();
index.add(&e).unwrap();
assert_eq!(index.get(0).unwrap().path, b"foo/bar");
assert_eq!(
index.get_path(Path::new("foo/bar"), 0).unwrap().path,
b"foo/bar"
);
assert_eq!(index.find_prefix(Path::new("foo2/")).unwrap(), Some(1));
assert_eq!(index.find_prefix(Path::new("empty/")).unwrap(), None);
}

#[test]
fn add_frombuffer_then_read() {
let (_td, repo) = crate::test::repo_init();
Expand Down