[PATCH 0/2 v2] Document update for 'git-blame' '-M' and '-C' option

[PATCH 0/2 v2] Document update for 'git-blame' '-M' and '-C' option

[Bo Yang]

The second version of the patches.

Bo Yang (2):
  Add a basic idea section for git-blame.
  Change the description of '-M' and '-C' option.

 Documentation/blame-options.txt |   46 +++++++++++++++++++++++---------------
 Documentation/git-blame.txt     |   35 ++++++++++++++++++++++++++++-
 2 files changed, 62 insertions(+), 19 deletions(-)

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

[Bo Yang]

Explain the basic idea about blame shifting with
'-M' or '-C' given.

Thanks-to: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 Documentation/git-blame.txt |   35 ++++++++++++++++++++++++++++++++++-
 1 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-blame.txt b/Documentation/git-blame.txt
index a27f439..3378665 100644
--- a/Documentation/git-blame.txt
+++ b/Documentation/git-blame.txt
@@ -9,7 +9,7 @@ SYNOPSIS
 --------
 [verse]
 'git blame' [-c] [-b] [-l] [--root] [-t] [-f] [-n] [-s] [-p] [-w] [--incremental] [-L n,m]
-	    [-S <revs-file>] [-M] [-C] [-C] [-C] [--since=<date>]
+	    [-S <revs-file>] [-M|<num>|] [-C|<num>|] [-C|<num>|] [-C|<num>|] [--since=<date>]
 	    [<rev> | --contents <file> | --reverse <rev>] [--] <file>
 
 DESCRIPTION
@@ -36,6 +36,39 @@ $ git log --pretty=oneline -S'blame_usage'
 ea4c7f9bf69e781dd0cd88d2bccb2bf5cc15c9a7 git-blame: Make the output
 -----------------------------------------------------------------------------
 
+
+BASIC IDEA
+----------
+
+This section briefly explains the basic idea behind 'git-blame'.  You
+do not have to understand it to use git-blame, but it helps in
+understanding the `-M` and `-C` options.  For the sake of simplicity,
+we assume that history is linear.
+
+A call to `git-blame <rev> -- <file>` works as follows:
+
+- Assume all the lines' blame to <rev> initially.
+
+- Run git diff <rev>^ <rev> and ignore all +/- lines. The unchanged
+  lines are definitely from our parent, so pass the blame of the
+  unchanged lines to parent.
+
+- For the +/- lines, take the blame if there are no `-M` or `-C`
+  options given. 
+
+- Repeat step 2~3 for all the remain lines which does not find
+  a blame until all lines find its blame.
+
+If there are `-M` or `-C` given, the command will try to search for
+same code of current lines and pass blame to it.
+
+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.
+
+
 OPTIONS
 -------
 include::blame-options.txt[]
-- 
1.7.0.2.273.gc2413.dirty

[PATCH 2/2 v2] Change the description of '-M' and '-C' option.

[Bo Yang]

Both '-M' and '-C' option detect code moving and copying.
The difference between the two options is whether they
search across file boundary.

Thanks-to: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Bo Yang <struggleyb.nku@gmail.com>
---
 Documentation/blame-options.txt |   46 +++++++++++++++++++++++---------------
 1 files changed, 28 insertions(+), 18 deletions(-)

diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
index 4833cac..d113f2e 100644
--- a/Documentation/blame-options.txt
+++ b/Documentation/blame-options.txt
@@ -79,34 +79,44 @@ 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 moving/copying lines in the file as well.  Instead of
+	taking blame for all '+' lines, attempt to find the same
+	lines 'in the same file' in the parent commit.  If such a
+	match was found, shift the blame to these lines in the
+	parent. (This expends extra effort on the order of the size
+	of the file for every change.)
++
+The net effect is that if code is moved or copied within the file, the
+lines are attributed to the original instead of the move/copy.
 +
 <num> is optional but it is the lower bound on the number of
 alphanumeric characters that git must detect as moving
 within a file for it to associate those lines with the parent
-commit.
+commit. And the default value is 20.
 
 -C|<num>|::
-	In addition to `-M`, detect lines 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,
-	the command additionally looks for copies from other
-	files in the commit that creates the file. When this
-	option is given three times, the command additionally
-	looks for copies from other files in any commit.
+	Like `-M`, detect moving/copying lines between files as well.
+	Instead of taking blame for all '+' lines, attempt to find the same
+	lines across file boundary according to the number of given `-C`.
+	This is useful when you reorganize your program and move/copy
+	code around across files. When this option is given once, detect
+	lines moved/copied from other files that were modified in the
+	same commit. (This expends extra effort on the order of
+	<number of modified files>*<file size> for every change.) When this
+	option is given twice, the command additionally looks for moves/copies
+	from other files in the commit that creates this file. (This expends
+	extra effort on the order of <number of files in the commit>*<file size>
+	for every change.) When this option is given three times, the
+	command additionally looks for moves/copies from other files
+	in any commit. (This expends extra effort on the order of
+	<commit number>*<number of files in one commit>*<file size>
+	for every change.)
 +
 <num> is optional but it is the lower bound on the number of
 alphanumeric characters that git must detect as moving
 between files for it to associate those lines with the parent
-commit.
+commit. And the default value is 40. If there are different values
+provided by different `-C` option, the last value will take effect finally.
 
 -h::
 --help::
-- 
1.7.0.2.273.gc2413.dirty

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

[Junio C Hamano]

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
+	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,

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

[Bo Yang]

Hi Junio,

On Sun, Apr 11, 2010 at 3:53 AM, Junio C Hamano <gitster@pobox.com> wrote:
> 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?

I think all the four things above are mentioned in [PATCH 2/2 v2]
message, it contains who should the command pass blame to and the
order of the algorithm used. Would you please take a look at that
patch?

And the BASIC IDEA section just want to make a basic description about
how blame works briefly. If you thought that it is non-use for
end-users, how about just discard it and make a more technical one at
technical/git-blame.txt ?
Thanks!

Regards!
Bo

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

[Pete Harlan]

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,