Something wrong with pickaxe?

Something wrong with pickaxe?

[Johannes Schindelin]

Hi,

it is a fact that I'm stupid, but in this case it might be a bug, too. 
When I call

	git-whatchanged -Sget_remote_heads 1baaae5e fetch-pack.c

the first shown commit is d1c133f5. However, if I substitute the 
"-Sget_remote_heads" by "-p" I see that the diff of commit 1baaae5e *does* 
change a line containing that text.

What am I doing wrong?

Ciao,
Dscho

Re: Something wrong with pickaxe?

[Linus Torvalds]

On Thu, 19 Jan 2006, Johannes Schindelin wrote:
> 
> it is a fact that I'm stupid, but in this case it might be a bug, too. 
> When I call
> 
> 	git-whatchanged -Sget_remote_heads 1baaae5e fetch-pack.c
> 
> the first shown commit is d1c133f5. However, if I substitute the 
> "-Sget_remote_heads" by "-p" I see that the diff of commit 1baaae5e *does* 
> change a line containing that text.
> 
> What am I doing wrong?

Nothing. But pickaxe really has very strange semantics, which imho makes 
it much less useful than it could be.

What pickaxe ends up doing is to literally see if the original has that 
string, and the result does not. I think it _counts_ the number of 
occurrences of a string in the before/after situation, and if the count 
differs, it's considered interesting.

So yes, there's a commit that has "get_remote_heads" as part of the 
change, but that particular string did not actually change in that commit: 
_other_ stuff on the same line did change. That particular string existed 
both before and after.

So when you use pickaxe, you really want to match the whole line you're 
looking for - otherwise you'll only see when people add or remove a 
particular string, not when they change things around it. Even then, if 
that particular line gets _moved_ (but otherwise is unchanged) pickaxe 
won't pick it up.

So you could have done a more exact search:

    git-whatchanged -p \
	-S"get_remote_heads(fd[0], &ref, nr_match, match, 1);" \
	1baaae5e fetch-pack.c

would have found where that particular line was introduced (and deleted).

Me, I find the pickaxe semantics so non-intuitive that I never use it 
(that said, the counting begaviour is better than what it _used_ to be, 
which, if I recall correctly, was just "it existed before, doesn't exist 
now").

I at one point suggested to Junio that the semantics be something else 
("mark the source and destination within 'x' characters of an occurrence 
of that string, and then if the _delta_ touches any of the marked areas, 
consider it to be a hit") but I think the problem was simply that it's 
more complex. You have to look at the ranges that the delta actually 
touches, which is more work than just looking at the original and final 
file contents themselves.

I suspect (but can't speak for him, obviously) that Junio would be open to 
more friendly pickaxe semantics if the suggested semantics change was 
accompanied by an actual diff to implement them.

In the meantime, you really need to think carefully about what you're 
doing when you use "-S". 

		Linus

Re: Something wrong with pickaxe?

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> So when you use pickaxe, you really want to match the whole line you're 
> looking for - otherwise you'll only see when people add or remove a 
> particular string, not when they change things around it. Even then, if 
> that particular line gets _moved_ (but otherwise is unchanged) pickaxe 
> won't pick it up.

True.  I always feed at least couple of lines to it.

> ... Junio would be open to 
> more friendly pickaxe semantics if the suggested semantics change was 
> accompanied by an actual diff to implement them.

True again.  It is hard to be "more friendly" without actually
generating a diff ;-).

Re: Something wrong with pickaxe?

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> Me, I find the pickaxe semantics so non-intuitive that I never use it 
> (that said, the counting begaviour is better than what it _used_ to be, 
> which, if I recall correctly, was just "it existed before, doesn't exist 
> now").

You are so polite to say "non-intuitive" when you mean
"useless".

I use pickaxe primarily when I want to find out what commit made
the code into the current shape.

For example, after I start debugging or code change, if I
suspect the first "if (!nr_match) return;" is bogus in
filter_refs() of fetch-pack.c, before declaring that code is
bogus and start butchering it, I run something like this:

$ git whatchanged -p -S'static void filter_refs(struct ref **refs, int nr_match, char **match)
{
	struct ref *prev, *current, *next;

	if (!nr_match)
		return;
' master -- fetch-pack.c

to find the commit that made the function to look like so,
hoping to find the reason _why_ it is that way is explained in
the log message for that commit.  If the commit log is not
enough, then next thing I do is to see its commit/author date
and go back to the mailing list logs to see discussion around
that timeframe.

For this use pattern, even counting behaviour we added later is
redundant, since "did not have it that way but now it does" is
the only thing that matters.

"What commits changed the lines that use this symbol over time"
is not a question I ask pickaxe.  When I ask a question about an
individual symbol, such as "get_remote_heads", I often use "git
grep", to learn what other existing users of the function would
be affected in the current code if I changed that function, or
to learn how other functions in the current code uses that
symbol, but for that kind of research, historical usage of that
symbol does not interest me.

Having said all of the above, I agree a different kind of
pickaxe that gives you "if you were to run diff -u between these
two blobs, the resulting patch would contain this text" might
also be useful, and I am very open to have such an additional
diffcore module next to pickaxe (IOW, not a replacement).

But it might not be as easy to use as the current pickaxe if the
question I want to ask is "who made it into this shape".

Re: Something wrong with pickaxe?

[Linus Torvalds]

On Wed, 18 Jan 2006, Junio C Hamano wrote:
> 
> You are so polite to say "non-intuitive" when you mean
> "useless".

No, no. I could imagine that it's useful, but I end up always instead 
doing "git whatchanged | less" and searching for it. 

So I really mean that the semantics are so non-intuitive that I just can't 
use it, rather than totally useless.

> For example, after I start debugging or code change, if I
> suspect the first "if (!nr_match) return;" is bogus in
> filter_refs() of fetch-pack.c, before declaring that code is
> bogus and start butchering it, I run something like this:
> 
> $ git whatchanged -p -S'static void filter_refs(struct ref **refs, int nr_match, char **match)
> {
> 	struct ref *prev, *current, *next;
> 
> 	if (!nr_match)
> 		return;
> ' master -- fetch-pack.c

See, this is what I mean - it looks useful, but it's just so subtle that I 
don't ever use it.

For example, getting the tabs/spaces right is basically impossible with 
normal cut-and-paste. I assume you do the above inside GNU emacs or some 
similar special-purpose setup, rather than anything generic.

		Linus

Re: Something wrong with pickaxe?

[Junio C Hamano]

Linus Torvalds <torvalds@osdl.org> writes:

> For example, getting the tabs/spaces right is basically impossible with 
> normal cut-and-paste. I assume you do the above inside GNU emacs or some 
> similar special-purpose setup, rather than anything generic.

Ah, that is very very very true.  Sometimes I cut and paste
inside screen and find it annoying to see pasted tabs do not
work properly, so I understand that issue very well.

But most of the time, I always work inside Emacs and rarely step
out if ever.  Thanks for making me realize that.

Re: Something wrong with pickaxe?

[Andreas Ericsson]

Junio C Hamano wrote:
> Linus Torvalds <torvalds@osdl.org> writes:
> 
>>... Junio would be open to 
>>more friendly pickaxe semantics if the suggested semantics change was 
>>accompanied by an actual diff to implement them.
> 
> 
> True again.  It is hard to be "more friendly" without actually
> generating a diff ;-).
> 

I thought generating diffs was fairly cheap. I agree with Linus somewhat 
though. I've been puzzled more than once with the pickaxe behaviour when 
I give it a function name to search for and want to know when the 
calling semantics changed for that function. I usually also end up doing 
a whatchanged -p and search it.

It would be neat if the current behaviour could be kept with 
--pickaxe-exact but all commits containing changes to lines matching the 
string was returned by default. Or possibly --pickaxe-thourough to do 
the other way around.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231

Re: Something wrong with pickaxe?

[Junio C Hamano]

Andreas Ericsson <ae@op5.se> writes:

>> True again.  It is hard to be "more friendly" without actually
>> generating a diff ;-).
>
> I thought generating diffs was fairly cheap...

Probably.  But we currently do not do diff generation
internally, so the comparison is "now doing diff with N lines of
code" vs "having 0 lines of code to do diff".

> ... I give it a function name to search for and want to
> know when the calling semantics changed for that function.

"Calling semantics" meaning "function signature"?

Re: Something wrong with pickaxe?

[Andreas Ericsson]

Junio C Hamano wrote:
> Andreas Ericsson <ae@op5.se> writes:
> 
>>... I give it a function name to search for and want to
>>know when the calling semantics changed for that function.
> 
> 
> "Calling semantics" meaning "function signature"?
> 

Yes.

-- 
Andreas Ericsson                   andreas.ericsson@op5.se
OP5 AB                             www.op5.se
Tel: +46 8-230225                  Fax: +46 8-230231