From: Johan Herland Date: Mon, 11 Apr 2011 21:56:07 GMT Subject: Re: [PATCHv2 3/3] Teach --dirstat to not completely ignore rearranged lines within a file Message-ID: <201104112356.09408.johan@herland.net> In-Reply-To: <7vtye4tqmk.fsf@alter.siamese.dyndns.org> On Monday 11 April 2011, Junio C Hamano wrote: > Johan Herland writes: > > Currently, the --dirstat analysis fails to detect when lines within a > > file are rearranged, because the "damage" calculated by show_dirstat() > > is 0. However, if the SHA1 sum has changed, we already now that there > > should be at least some minimum amount of damage. > > This logic is sensible, modulo that "fails to detect" is actually > "ignores mere line movements on purpose". I apologize for my commit message not having caught up with discussion around this issue. I came into the discussion from the POV of "--dirstat does not pick up what --stat picks up; there must be a bug in --dirstat", and my original objective was therefore to "fix" --dirstat to be "more like --stat". Obviously, I now know exactly why --dirstat is different, and that we don't want to fundamentally change it. My commit message should have been rephrased in a more positive light as a result. Feel free to fix before applying. > In any case, if the object names are different, we already know that > there is _some_ damage, and it is very unintiutive to claim that there > is _no_ damage. Agreed. > > This patch teaches show_dirstat() to assign a minimum amount of damage > > (== 1) to entries for which the analysis otherwise yields zero damage. > > So it is perfectly in line with the above logic to give a minimum here. > Zero was simply just unintuitive, and this is a good fix to the problem. > > > Obviously this is not a complete fix, but it's at least better to > > I however do not understand what "a complete fix" means in this context. > You've fixed the unintuitiveness, and as far as the description in the > introductory paragraph of the problem goes, I think this already is a > complete fix. I still feel that a file with 1000 rearranged lines should somehow count "more" than a file with only 1 rearranged line, but it's hard to get there without futzing with diffcore_count_changes(), probably making the whole thing considerably more expensive... So in that sense, I agree that the current solution is probably as complete as we can get. ...Johan -- Johan Herland, www.herland.net