From: Junio C Hamano <hidden> Date: 2016-06-15 22:55:31
Morten Welinder [off-list ref] writes:
Is there a reason why picking among the choices in a sliding window
must be contents neutral?
Sorry, you might be getting at something interesting but I do not
understand the question. I have no idea what you mean by "contents
neutral".
Picking between these two choices
/** + /**
+ * Default parent + * Default parent
+ * + *
+ * @var int + * @var int
+ * @access protected + * @access protected
+ * @index + * @index
+ */ + */
+ protected $defaultParent; + protected $defaultParent;
+ +
+ /** /**
would not affect the correctness of the patch. You may pick
whatever you deem the most desirable, but your answer must be a
correct patch (the definition of "correct" here is "applying that
patch to the preimage produces the intended postimage").
And I think if you inserted a block of text B after a context C
where the tail of B matches the tail of C like the above, you can
shift what you treat as "inserted" up and still come up with a
correct patch.
The output being "a correct patch" is not the only thing we need to
consider, though, as I mentioned in another response to Kevin
regarding the "consequences".
From: Andrew Ardill <hidden> Date: 2016-06-15 22:55:31
On 13 December 2012 08:53, Junio C Hamano [off-list ref] wrote:
The output being "a correct patch" is not the only thing we need to
consider, though, as I mentioned in another response to Kevin
regarding the "consequences".
The main benefit of picking a more 'natural' diff is a usability one.
I know that when a chunk begins and ends one line after the logical
break point (typically with braces in my experience) mentally parsing
the diff becomes significantly harder. If there was a way to teach git
where it should try and break out a chunk (potentially per filetype?)
this is a good thing for readability, and I think would outweigh any
temporary pain with regards to cached rerere and diff data.
Regards,
Andrew Ardill
From: Javier Domingo <hidden> Date: 2016-06-15 22:55:31
I must say it is _quite_ helpfull having the diffs well done (natural
diffs as here named), just because when you want to review a patch on
the fly, this sort of things are annoying.
I just wanted to say my opinion. No idea on how to fix that, nor why
does it happen.
Javier Domingo
2012/12/12 Andrew Ardill [off-list ref]:
On 13 December 2012 08:53, Junio C Hamano [off-list ref] wrote:
quoted
The output being "a correct patch" is not the only thing we need to
consider, though, as I mentioned in another response to Kevin
regarding the "consequences".
The main benefit of picking a more 'natural' diff is a usability one.
I know that when a chunk begins and ends one line after the logical
break point (typically with braces in my experience) mentally parsing
the diff becomes significantly harder. If there was a way to teach git
where it should try and break out a chunk (potentially per filetype?)
this is a good thing for readability, and I think would outweigh any
temporary pain with regards to cached rerere and diff data.
Regards,
Andrew Ardill
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Michael Haggerty <hidden> Date: 2016-06-15 22:55:31
On 12/12/2012 10:53 PM, Junio C Hamano wrote:
Morten Welinder [off-list ref] writes:
quoted
Is there a reason why picking among the choices in a sliding window
must be contents neutral?
Sorry, you might be getting at something interesting but I do not
understand the question. I have no idea what you mean by "contents
neutral".
Picking between these two choices
/** + /**
+ * Default parent + * Default parent
+ * + *
+ * @var int + * @var int
+ * @access protected + * @access protected
+ * @index + * @index
+ */ + */
+ protected $defaultParent; + protected $defaultParent;
+ +
+ /** /**
would not affect the correctness of the patch. You may pick
whatever you deem the most desirable, but your answer must be a
correct patch (the definition of "correct" here is "applying that
patch to the preimage produces the intended postimage").
And I think if you inserted a block of text B after a context C
where the tail of B matches the tail of C like the above, you can
shift what you treat as "inserted" up and still come up with a
correct patch.
I have the feeling that a few crude heuristics would go a long way
towards improving diffs like this. For example:
* Prefer to have an add/remove block that has balanced begin/end pairs
(where begin/end pairs might be opening and closing parentheses,
brackets, braces, and angle brackets, "/*" and "*/", and perhaps a
couple of other things. For SGML-like text begin and end tags could be
matched up.
It would be possible to read these begin/end pairs from a
filetype-specific table or configuration setting, though this would add
complication and would also make it possible that diffs generated by two
different people are not identical if their configurations differ.
* Prefer to have a block where the first non-blank line of the block and
the first non-blank line after the block are indented by the same amount.
* Prefer to have a block with trailing (as opposed to leading or
embedded) blank lines--the more the better.
The beautiful thing is that even if the heuristics sometimes fail, the
correctness of the patch (in the sense that you have defined) is not
compromised.
Michael
--
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Is there a reason why picking among the choices in a sliding window
must be contents neutral?
Sorry, you might be getting at something interesting but I do not
understand the question. I have no idea what you mean by "contents
neutral".
I was merely asking if an algorithm to pick between the
2+ choices was allowed to look at the contents of the
lines.
I.e., an algorithm would look at the C comment
example and determine that the choice starting containing
a full inserted comment is preferable over the one that
appears to close one comment and open a new.
And the in inserted-function case it would prefer the one
where the matching { and } are in correct order.
Morten
On Dec 12, 2012, at 20:55, Morten Welinder [off-list ref] wrote:
I was merely asking if an algorithm to pick between the
2+ choices was allowed to look at the contents of the
lines.
I.e., an algorithm would look at the C comment
example and determine that the choice starting containing
a full inserted comment is preferable over the one that
appears to close one comment and open a new.
And the in inserted-function case it would prefer the one
where the matching { and } are in correct order.
/** + /**
+ * Default parent + * Default parent
+ * + *
+ * @var int + * @var int
+ * @access protected + * @access protected
+ * @index + * @index
+ */ + */
+ protected $defaultParent; + protected $defaultParent;
+ +
+ /** /**
It would seem that just looking at the line length (stripped) of
the last line, might be sufficient for cost function to minimize.
Here the some would be 3 vs 0. In case of ties, use the last
possibility with minimum cost.
I think it would be nice if the cost function we choose does not
depend on file type, as that is something that is very dependent
on the exact local configuration and might hinder comparison of
patches. If something really simple gets us 90% there, that would
be preferable over extra complexity.
-Geert
Junio's other example:
}
+void new_function(void)
+{
+ printf("hello, world.\n");
+}
+
void existing_one(void)
{
printf("goodbye, world.\n");
=> Cost 0
+}
+
+void new_function(void)
+{
+ printf("hello, world.\n");
}
=> Cost 27
Kevin's example:
/**
+ * Default parent
+ *
+ * @var int
+ * @access protected
+ * @index
+ */
+ protected $defaultParent;
+
+ /**
=> Cost 3
+ /**
+ * Default parent
+ *
+ * @var int
+ * @access protected
+ * @index
+ */
+ protected $defaultParent;
+
/**
=> cost 0