Pieter de Bie [off-list ref] writes:
On Aug 10, 2008, at 3:01 AM, Junio C Hamano wrote:
quoted
- if (!dwim_ref(argv[i], spec - argv[i], sha1, &ref)) {
+ if (!dwim_log(argv[i], spec - argv[i], sha1, &ref)) {
This is also what add_reflog_for_walk() does, but that function tries to resolve
the argv[i] part first, without doing the dwim_log().
Perhaps we can also ...
Sorry, I do not understand what you meant by the above comment.
- "This is also what add_reflog_for_walk() does" -- I take it you mean
the use of dwim_log() instead of dwim_ref()?
- "... but that function tries to resolve the argv[i] part first" -- do
you mean the resolve_ref("HEAD"...) call inside "if (!*branch)"
codepath?
That one serves different purposes than "delete HEAD@{42}". It is
about showing "@{42}" --- in order to show reflog for "the current
branch", it figures out the current branch by resolving "HEAD".
In any case, what confuses me is I cannot tell if you do or do not have
issues that I did not think of with the "s/dwim_ref/dwim_log/" change.
Are you saying "no that cannot be a correct fix; see the way dwim_log() is
used in add_reflog_for_walk() -- it does more than your one-liner"?
By the way, I think the idea of "Perhaps we can also..." part is good.
On Aug 10, 2008, at 8:52 PM, Junio C Hamano wrote:
Pieter de Bie [off-list ref] writes:
quoted
On Aug 10, 2008, at 3:01 AM, Junio C Hamano wrote:
quoted
- if (!dwim_ref(argv[i], spec - argv[i], sha1, &ref)) {
+ if (!dwim_log(argv[i], spec - argv[i], sha1, &ref)) {
This is also what add_reflog_for_walk() does, but that function
tries to resolve
the argv[i] part first, without doing the dwim_log().
Perhaps we can also ...
Sorry, I do not understand what you meant by the above comment.
Sorry, it was early in the morning ;)
- "This is also what add_reflog_for_walk() does" -- I take it you mean
the use of dwim_log() instead of dwim_ref()?
Yes
- "... but that function tries to resolve the argv[i] part first" --
do
you mean the resolve_ref("HEAD"...) call inside "if (!*branch)"
codepath?
That one serves different purposes than "delete HEAD@{42}". It is
about showing "@{42}" --- in order to show reflog for "the current
branch", it figures out the current branch by resolving "HEAD".
No, I meant this part:
reflogs = read_complete_reflog(branch);
if (!reflogs || reflogs->nr == 0)
if (dwim_log(branch, strlen(branch), sha1, &b) == 1) {
branch = b;
reflogs = read_complete_reflog(branch);
}
Which seems to suggest that the read_complete_reflog() may produce
different results if dwim_log() is not called. However, I did not
follow the codepath to see why.
In any case, what confuses me is I cannot tell if you do or do not
have
issues that I did not think of with the "s/dwim_ref/dwim_log/" change.
Are you saying "no that cannot be a correct fix; see the way
dwim_log() is
used in add_reflog_for_walk() -- it does more than your one-liner"?
I think the change looks ok. The only 'problem' I had was the chunk
above, because I do not know if the double call to
read_complete_reflog, once without a dwim_log and optionally once
with, is significant. However, I'm not familiar enough with the code
to make any observation other than that, which is why my reply had no
conclusion ;)
Hope that clears things up.
By the way, I think the idea of "Perhaps we can also..." part is good.
I'll send in a better patch.
- Pieter
This allows you to specify 'git reflog expire master' without needing
to give the full refname like 'git reflog expire refs/heads/master'
Signed-off-by: Pieter de Bie <redacted>
---
builtin-reflog.c | 8 ++++----
1 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/builtin-reflog.c b/builtin-reflog.c
index 5af3f28..f4d1f32 100644
--- a/builtin-reflog.c
+++ b/builtin-reflog.c
@@ -540,11 +540,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)
free(collected.e);
}
- while (i < argc) {
- const char *ref = argv[i++];
+ for (; i < argc; i++) {
+ char *ref;
unsigned char sha1[20];
- if (!resolve_ref(ref, sha1, 1, NULL)) {
- status |= error("%s points nowhere!", ref);
+ if (!dwim_log(argv[i], strlen(argv[i]), sha1, &ref)) {
+ status |= error("%s points nowhere!", argv[i]);
continue;
}
set_reflog_expiry_param(&cb, explicit_expiry, ref);--
1.6.0.rc0.320.g49281