Commit b15df1d937128bb7051ab35084195ff1980a7869

nulltoken 2012-11-17T18:29:51

reflog: make entry_byindex() and drop() git compliant Passing 0 as the index now retrieves the most recent entry instead of the oldest one.

diff --git a/include/git2/reflog.h b/include/git2/reflog.h
index 20e0f33..72e1f17 100644
--- a/include/git2/reflog.h
+++ b/include/git2/reflog.h
@@ -88,8 +88,12 @@ GIT_EXTERN(unsigned int) git_reflog_entrycount(git_reflog *reflog);
 /**
  * Lookup an entry by its index
  *
+ * Requesting the reflog entry with an index of 0 (zero) will
+ * return the most recently created entry.
+ *
  * @param reflog a previously loaded reflog
- * @param idx the position to lookup
+ * @param idx the position of the entry to lookup. Should be greater than or
+ * equal to 0 (zero) and less than `git_reflog_entrycount()`.
  * @return the entry; NULL if not found
  */
 GIT_EXTERN(const git_reflog_entry *) git_reflog_entry_byindex(git_reflog *reflog, size_t idx);
@@ -103,7 +107,8 @@ GIT_EXTERN(const git_reflog_entry *) git_reflog_entry_byindex(git_reflog *reflog
  *
  * @param reflog a previously loaded reflog.
  *
- * @param idx the position of the entry to remove.
+ * @param idx the position of the entry to remove. Should be greater than or
+ * equal to 0 (zero) and less than `git_reflog_entrycount()`.
  *
  * @param rewrite_previous_entry 1 to rewrite the history; 0 otherwise.
  *
@@ -112,7 +117,7 @@ GIT_EXTERN(const git_reflog_entry *) git_reflog_entry_byindex(git_reflog *reflog
  */
 GIT_EXTERN(int) git_reflog_drop(
 	git_reflog *reflog,
-	unsigned int idx,
+	size_t idx,
 	int rewrite_previous_entry);
 
 /**
diff --git a/src/reflog.c b/src/reflog.c
index 0e333aa..7b07c6a 100644
--- a/src/reflog.c
+++ b/src/reflog.c
@@ -291,8 +291,8 @@ success:
 int git_reflog_append(git_reflog *reflog, const git_oid *new_oid,
 				const git_signature *committer, const char *msg)
 {
-	int count;
 	git_reflog_entry *entry;
+	const git_reflog_entry *previous;
 	const char *newline;
 
 	assert(reflog && new_oid && committer);
@@ -319,16 +319,12 @@ int git_reflog_append(git_reflog *reflog, const git_oid *new_oid,
 		}
 	}
 
-	count = git_reflog_entrycount(reflog);
+	previous = git_reflog_entry_byindex(reflog, 0);
 
-	if (count == 0)
+	if (previous == NULL)
 		git_oid_fromstr(&entry->oid_old, GIT_OID_HEX_ZERO);
-	else {
-		const git_reflog_entry *previous;
-
-		previous = git_reflog_entry_byindex(reflog, count -1);
+	else
 		git_oid_cpy(&entry->oid_old, &previous->oid_cur);
-	}
 
 	git_oid_cpy(&entry->oid_cur, new_oid);
 
@@ -417,8 +413,16 @@ unsigned int git_reflog_entrycount(git_reflog *reflog)
 
 const git_reflog_entry * git_reflog_entry_byindex(git_reflog *reflog, size_t idx)
 {
+	int pos;
+
 	assert(reflog);
-	return git_vector_get(&reflog->entries, idx);
+
+	pos = git_reflog_entrycount(reflog) - (idx + 1);
+
+	if (pos < 0)
+		return NULL;
+
+	return git_vector_get(&reflog->entries, pos);
 }
 
 const git_oid * git_reflog_entry_oidold(const git_reflog_entry *entry)
@@ -447,7 +451,7 @@ char * git_reflog_entry_msg(const git_reflog_entry *entry)
 
 int git_reflog_drop(
 	git_reflog *reflog,
-	unsigned int idx,
+	size_t idx,
 	int rewrite_previous_entry)
 {
 	unsigned int entrycount;
@@ -457,30 +461,31 @@ int git_reflog_drop(
 
 	entrycount = git_reflog_entrycount(reflog);
 
-	if (idx >= entrycount)
+	entry = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx);
+
+	if (entry == NULL)
 		return GIT_ENOTFOUND;
 
-	entry = git_vector_get(&reflog->entries, idx);
 	reflog_entry_free(entry);
 
-	if (git_vector_remove(&reflog->entries, idx) < 0)
+	if (git_vector_remove(&reflog->entries, entrycount - (idx + 1)) < 0)
 		return -1;
 
 	if (!rewrite_previous_entry)
 		return 0;
 
 	/* No need to rewrite anything when removing the most recent entry */
-	if (idx == entrycount - 1)
+	if (idx == 0)
 		return 0;
 
-	/* There are no more entries in the log */
+	/* Have the latest entry just been dropped? */
 	if (entrycount == 1)
 		return 0;
 
-	entry = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx);
+	entry = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx - 1);
 
 	/* If the oldest entry has just been removed... */
-	if (idx == 0) {
+	if (idx == entrycount - 1) {
 		/* ...clear the oid_old member of the "new" oldest entry */
 		if (git_oid_fromstr(&entry->oid_old, GIT_OID_HEX_ZERO) < 0)
 			return -1;
@@ -488,7 +493,7 @@ int git_reflog_drop(
 		return 0;
 	}
 
-	previous = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx - 1);
+	previous = (git_reflog_entry *)git_reflog_entry_byindex(reflog, idx);
 	git_oid_cpy(&entry->oid_old, &previous->oid_cur);
 
 	return 0;
diff --git a/src/revparse.c b/src/revparse.c
index 83eea7d..6a7587d 100644
--- a/src/revparse.c
+++ b/src/revparse.c
@@ -201,7 +201,7 @@ static int retrieve_previously_checked_out_branch_or_revision(git_object **out, 
 
 	numentries  = git_reflog_entrycount(reflog);
 
-	for (i = numentries - 1; i >= 0; i--) {
+	for (i = 0; i < numentries; i++) {
 		entry = git_reflog_entry_byindex(reflog, i);
 		msg = git_reflog_entry_msg(entry);
 		
@@ -263,7 +263,7 @@ static int retrieve_oid_from_reflog(git_oid *oid, git_reference *ref, unsigned i
 		}
 
 		entry = git_reflog_entry_byindex(reflog, identifier);
-		git_oid_cpy(oid, git_reflog_entry_oidold(entry));
+		git_oid_cpy(oid, git_reflog_entry_oidnew(entry));
 		error = 0;
 		goto cleanup;
 
@@ -271,7 +271,7 @@ static int retrieve_oid_from_reflog(git_oid *oid, git_reference *ref, unsigned i
 		int i;
 		git_time commit_time;
 
-		for (i = numentries - 1; i >= 0; i--) {
+		for (i = 0; i < numentries; i++) {
 			entry = git_reflog_entry_byindex(reflog, i);
 			commit_time = git_reflog_entry_committer(entry)->when;
 					
diff --git a/src/stash.c b/src/stash.c
index 627d271..b74429a 100644
--- a/src/stash.c
+++ b/src/stash.c
@@ -600,7 +600,7 @@ int git_stash_foreach(
 
 	max = git_reflog_entrycount(reflog);
 	for (i = 0; i < max; i++) {
-		entry = git_reflog_entry_byindex(reflog, max - i - 1);
+		entry = git_reflog_entry_byindex(reflog, i);
 		
 		if (callback(i,
 			git_reflog_entry_msg(entry),
@@ -642,7 +642,7 @@ int git_stash_drop(
 		goto cleanup;
 	}
 
-	if ((error = git_reflog_drop(reflog, max - index - 1, true)) < 0)
+	if ((error = git_reflog_drop(reflog, index, true)) < 0)
 		goto cleanup;
 
 	if ((error = git_reflog_write(reflog)) < 0)
diff --git a/tests-clar/refs/reflog/drop.c b/tests-clar/refs/reflog/drop.c
index 4be857b..512e8ba 100644
--- a/tests-clar/refs/reflog/drop.c
+++ b/tests-clar/refs/reflog/drop.c
@@ -49,14 +49,16 @@ void test_refs_reflog_drop__can_drop_an_entry_and_rewrite_the_log_history(void)
 
 	cl_assert(entrycount > 4);
 
-	before_current = git_reflog_entry_byindex(g_reflog, 2);
+	before_current = git_reflog_entry_byindex(g_reflog, 1);
+
 	git_oid_cpy(&before_current_old_oid, &before_current->oid_old);
 	git_oid_cpy(&before_current_cur_oid, &before_current->oid_cur);
 
-	cl_git_pass(git_reflog_drop(g_reflog, 2, 1));
+	cl_git_pass(git_reflog_drop(g_reflog, 1, 1));
+
 	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
 
-	after_current = git_reflog_entry_byindex(g_reflog, 2);
+	after_current = git_reflog_entry_byindex(g_reflog, 0);
 
 	cl_assert_equal_i(0, git_oid_cmp(&before_current_old_oid, &after_current->oid_old));
 	cl_assert(0 != git_oid_cmp(&before_current_cur_oid, &after_current->oid_cur));
@@ -68,10 +70,10 @@ void test_refs_reflog_drop__can_drop_the_oldest_entry(void)
 
 	cl_assert(entrycount > 2);
 
-	cl_git_pass(git_reflog_drop(g_reflog, 0, 0));
+	cl_git_pass(git_reflog_drop(g_reflog, entrycount - 1, 0));
 	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
 
-	entry = git_reflog_entry_byindex(g_reflog, 0);
+	entry = git_reflog_entry_byindex(g_reflog, entrycount - 2);
 	cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) != 0);
 }
 
@@ -81,10 +83,10 @@ void test_refs_reflog_drop__can_drop_the_oldest_entry_and_rewrite_the_log_histor
 
 	cl_assert(entrycount > 2);
 
-	cl_git_pass(git_reflog_drop(g_reflog, 0, 1));
+	cl_git_pass(git_reflog_drop(g_reflog, entrycount - 1, 1));
 	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
 
-	entry = git_reflog_entry_byindex(g_reflog, 0);
+	entry = git_reflog_entry_byindex(g_reflog, entrycount - 2);
 	cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0);
 }
 
@@ -93,8 +95,8 @@ void test_refs_reflog_drop__can_drop_all_the_entries(void)
 	cl_assert(--entrycount > 0);
 
 	do 	{
-		cl_git_pass(git_reflog_drop(g_reflog, --entrycount, 1));
-	} while (entrycount > 0);
+		cl_git_pass(git_reflog_drop(g_reflog, 0, 1));
+	} while (--entrycount > 0);
 
 	cl_git_pass(git_reflog_drop(g_reflog, 0, 1));
 
@@ -108,7 +110,7 @@ void test_refs_reflog_drop__can_persist_deletion_on_disk(void)
 	cl_assert(entrycount > 2);
 
 	cl_git_pass(git_reference_lookup(&ref, g_repo, g_reflog->ref_name));
-	cl_git_pass(git_reflog_drop(g_reflog, entrycount - 1, 1));
+	cl_git_pass(git_reflog_drop(g_reflog, 0, 1));
 	cl_assert_equal_i(entrycount - 1, git_reflog_entrycount(g_reflog));
 	cl_git_pass(git_reflog_write(g_reflog));
 
diff --git a/tests-clar/refs/reflog/reflog.c b/tests-clar/refs/reflog/reflog.c
index 20f08f5..09b9356 100644
--- a/tests-clar/refs/reflog/reflog.c
+++ b/tests-clar/refs/reflog/reflog.c
@@ -68,13 +68,13 @@ void test_refs_reflog_reflog__append_then_read(void)
 	cl_git_pass(git_reflog_read(&reflog, lookedup_ref));
 	cl_assert_equal_i(2, git_reflog_entrycount(reflog));
 
-	entry = git_reflog_entry_byindex(reflog, 0);
+	entry = git_reflog_entry_byindex(reflog, 1);
 	assert_signature(committer, entry->committer);
 	cl_assert(git_oid_streq(&entry->oid_old, GIT_OID_HEX_ZERO) == 0);
 	cl_assert(git_oid_cmp(&oid, &entry->oid_cur) == 0);
 	cl_assert(entry->msg == NULL);
 
-	entry = git_reflog_entry_byindex(reflog, 1);
+	entry = git_reflog_entry_byindex(reflog, 0);
 	assert_signature(committer, entry->committer);
 	cl_assert(git_oid_cmp(&oid, &entry->oid_old) == 0);
 	cl_assert(git_oid_cmp(&oid, &entry->oid_cur) == 0);
diff --git a/tests-clar/stash/drop.c b/tests-clar/stash/drop.c
index 39139cc..b4f73b9 100644
--- a/tests-clar/stash/drop.c
+++ b/tests-clar/stash/drop.c
@@ -99,7 +99,7 @@ void test_stash_drop__dropping_an_entry_rewrites_reflog_history(void)
 	cl_git_pass(git_stash_drop(repo, 1));
 
 	cl_git_pass(git_reflog_read(&reflog, stash));
-	entry = git_reflog_entry_byindex(reflog, 1);
+	entry = git_reflog_entry_byindex(reflog, 0);
 
 	cl_assert_equal_i(0, git_oid_cmp(&oid, git_reflog_entry_oidold(entry)));
 	cl_assert_equal_i(count - 1, git_reflog_entrycount(reflog));