Re: [PATCH 1/2 v2] Add a basic idea section for git-blame.
On 04/10/2010 12:53 PM, Junio C Hamano wrote:
Show 64 quoted lines
> 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
Show 17 quoted lines
> + 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,