Re: [PATCH 1/2 v2] Add a basic idea section for git-blame.

[Pete Harlan <pgit@pcharlan.com>]

On 04/10/2010 12:53 PM, Junio C Hamano wrote:
> Bo Yang <struggleyb.nku@gmail.com> writes:
> 
>> +With `-M`, this command detects same lines of the current blaming code
>> +inside the current file. And it will shift the blame to the author of
>> +the original lines instead of author of current blaming code. It does
>> +the same for `-C` except that it will search across file boundary and
>> +multiple commits.
> 
> I grant you that the understanding what M/C options do by the end users
> (the target audience of the document) would improve if they understood the
> above paragraph.  And I know you thought the text leading to the above
> paragraph (omitted) would help them understand what this paragraph tells
> them.
> 
> But I think we should try to do better.  We can always say "With a
> technical description of how it works internally, you can understand why
> these options give you the behaviour you want", but that should be the
> last resort when we cannot give meaningful description without going into
> the implementation details.
> 
> It may also help git hacker wannabes (not end users) to have more detailed
> and precise description of how the algorithm works in a separate document
> in the Documentation/technical/ area, but that is a separate issue.
> 
> If the goal is to help the end users use M/C options and understand the
> output better, can't we take a more direct approach?
> 
> It doesn't really matter to them _why_ only B is blamed to the parent in
> "A B" -> "B A" movement without -M (and your "BASIC IDEA" section is too
> sketchy for readers to guess why, even if they wanted to learn the
> implementation detail, which they typically don't).
> 
> Things like:
> 
>     - they can use -M to annotate across block-of-lines swappage within a file;
>     - use of -M adds cost --- it spends extra cycles;
>     - similarly -C annotates across block-of-lines swappage between files;
>     - use -f -C adds larger cost; ...
> 
> are the only important things they want to know about, no?
> 
>  Documentation/blame-options.txt |   19 ++++++++++---------
>  1 files changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> index 4833cac..5d5ed37 100644
> --- a/Documentation/blame-options.txt
> +++ b/Documentation/blame-options.txt
> @@ -79,14 +79,15 @@ of lines before or after the line given by <start>.
>  	of the --date option at linkgit:git-log[1].
>  
>  -M|<num>|::
> -	Detect moving lines in the file as well.  When a commit
> -	moves a block of lines in a file (e.g. the original file
> -	has A and then B, and the commit changes it to B and
> -	then A), the traditional 'blame' algorithm typically blames
> -	the lines that were moved up (i.e. B) to the parent and
> -	assigns blame to the lines that were moved down (i.e. A)
> -	to the child commit.  With this option, both groups of lines
> -	are blamed on the parent.
> +	Detect moved or copied lines within a file. When a commit
> +	moves or copies a block of lines (e.g. the original file
> +	has A and then B, and the commit changes it to B and then
> +	A), the traditional 'blame' algorithm notices only the

There's an extraneous "the" at the end of this line.

Other than that everything you say here sounds like a good idea to me.

--Pete

> +	half of the movement and typically blames the lines that were
> +	moved up (i.e. B) to the parent and assigns blame to the lines
> +	that were moved down (i.e. A) to the child commit.  With this
> +	option, both groups of lines are blamed on the parent by
> +	running extra passes of inspection.
>  +
>  <num> is optional but it is the lower bound on the number of
>  alphanumeric characters that git must detect as moving
> @@ -94,7 +95,7 @@ within a file for it to associate those lines with the parent
>  commit.
>  
>  -C|<num>|::
> -	In addition to `-M`, detect lines copied from other
> +	In addition to `-M`, detect lines moved or copied from other
>  	files that were modified in the same commit.  This is
>  	useful when you reorganize your program and move code
>  	around across files.  When this option is given twice,