On Mon, Mar 28, 2016 at 4:28 PM, Stefan Beller wrote:
> cat > expect < +Entering '../nested1'
> +Entering '../nested1/nested2'
> +Entering '../nested1/nested2/nested3'
> +Entering '../nested1/nested2/nested3/submodule'
> +Entering '../sub1'
> +Entering '../sub2'
> +Entering '../sub3'
> +EOF
> +
>
On Mon, Mar 28, 2016 at 5:26 PM, Jacob Keller wrote:
> On Mon, Mar 28, 2016 at 4:28 PM, Stefan Beller wrote:
>> cat > expect <> +Entering '../nested1'
>> +Entering '../nested1/nested2'
>> +Entering '../nested1/nested2/nested3'
>> +Entering '../nested1/nested2/nested3/submodule'
>> +Entering '../
Stefan Beller writes:
> I thought this is an optimization for C code where you have a diff like:
>
> int existingStuff1(..) {
> ...
> }
> +
> + int foo(..) {
> +...
> +}
>
> int existingStuff2(...) {
> ...
>
> Note that the closing '}' could be taken from the m
On Tue, Mar 29, 2016 at 10:54 AM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> I thought this is an optimization for C code where you have a diff like:
>>
>> int existingStuff1(..) {
>> ...
>> }
>> +
>> + int foo(..) {
>> +...
>> +}
>>
>> int existingStuff2(.
On Tue, Mar 29, 2016 at 11:16 AM, Stefan Beller wrote:
> On Tue, Mar 29, 2016 at 10:54 AM, Junio C Hamano wrote:
>> Stefan Beller writes:
>>
>>> I thought this is an optimization for C code where you have a diff like:
>>>
>>> int existingStuff1(..) {
>>> ...
>>> }
>>> +
>>> +
Jacob Keller writes:
> On Tue, Mar 29, 2016 at 11:16 AM, Stefan Beller wrote:
>> ...
>> To find a heuristic, which appeals both the C code
>> and the shell code, we could take the empty line
>> as a strong hint for the divider:
>
> This seems like a good heuristic. Can we think of any examples w
On Tue, Mar 29, 2016 at 04:05:57PM -0700, Jacob Keller wrote:
> > This is what we want in both cases.
> > And I would argue it would appease many other kinds of text as well, because
> > an empty line is usually a strong indicator for any text that a
> > different thing comes along.
> > (Other pro
On Tue, Mar 29, 2016 at 9:55 PM, Jeff King wrote:
> On Tue, Mar 29, 2016 at 04:05:57PM -0700, Jacob Keller wrote:
>
>> > This is what we want in both cases.
>> > And I would argue it would appease many other kinds of text as well,
>> > because
>> > an empty line is usually a strong indicator for
On Tue, Mar 29, 2016 at 9:55 PM, Jeff King wrote:
> One thing I like to do when playing with new diff ideas is to pipe all
> of "log -p" for a real project through it and see what differences it
> produces.
>
Great idea!
> Below is a perl script that implements Stefan's heuristic. I checked its
On Tue, Mar 29, 2016 at 11:05 PM, Jacob Keller wrote:
> On Tue, Mar 29, 2016 at 9:55 PM, Jeff King wrote:
>> One thing I like to do when playing with new diff ideas is to pipe all
>> of "log -p" for a real project through it and see what differences it
>> produces.
>>
>
> Great idea!
>
>> Below i
On Wed, Mar 30, 2016 at 12:14 PM, Jacob Keller wrote:
> I ran this on a few of my local projects and it doesn't seem to
> produce any false positives so far. Everything looks good. Of course
> this is with just traditional C code. I am currently trying to run
> this against the history of Linux as
On Wed, Mar 30, 2016 at 12:31 PM, Jacob Keller wrote:
>
> If unsure, say Y.
> +
> +config RMI4_I2C
> + tristate "RMI4 I2C Support"
> + depends on RMI4_CORE && I2C
> + help
> + Say Y here if you want to support RMI4 devices connected to an I2C
> + bus.
>
On Wed, Mar 30, 2016 at 12:31:30PM -0700, Jacob Keller wrote:
> So far I've only found a single location that ends up looking worse
> within the Linux kernel. Diffs of some Kbuild settings result in
> something like
>
> before:
>
> If unsure, say Y.
> +
> +config RMI4_I2C
> + tri
Stefan Beller writes:
> Thanks for going through examples!
> (I would, too. But fixing a submodule regression is more important
> now; I only develop new features when there are no known regressions
> caused by me)
This is a tangent but perhaps as an experiment perhaps we can try it
as the rule
On Thu, Mar 31, 2016 at 6:47 AM, Jeff King wrote:
> On Wed, Mar 30, 2016 at 12:31:30PM -0700, Jacob Keller wrote:
>
>> So far I've only found a single location that ends up looking worse
>> within the Linux kernel. Diffs of some Kbuild settings result in
>> something like
>>
>> before:
>>
>>
On Wed, Apr 6, 2016 at 10:47 AM, Jacob Keller wrote:
>
> I started attempting to implement this heuristic within xdiff, but I
> am at a loss as to how xdiff actually works. I suspect this would go
> in xdi_change_compact or after it, but I really don't understand how
> xdiff represents the diffs a
On Tue, 12 Apr 2016, Stefan Beller wrote:
> On Wed, Apr 6, 2016 at 10:47 AM, Jacob Keller wrote:
> >
> > I started attempting to implement this heuristic within xdiff, but I
> > am at a loss as to how xdiff actually works. I suspect this would go
> > in xdi_change_compact or after it, but I reall
On Thu, Apr 14, 2016 at 06:56:39AM -0700, Davide Libenzi wrote:
> That was a zillions of years ago :) , but from a quick look at email
> thread, if you want to do it within xdiff, xdi_change_compact would be
> the place. The issue is knowing in which situations one diff look
> better than another
On Thu, Apr 14, 2016 at 11:34 AM, Jeff King wrote:
> On Thu, Apr 14, 2016 at 06:56:39AM -0700, Davide Libenzi wrote:
>
>> That was a zillions of years ago :) , but from a quick look at email
>> thread, if you want to do it within xdiff, xdi_change_compact would be
>> the place. The issue is knowi
On Thu, Apr 14, 2016 at 2:05 PM, Stefan Beller wrote:
> On Thu, Apr 14, 2016 at 11:34 AM, Jeff King wrote:
>> On Thu, Apr 14, 2016 at 06:56:39AM -0700, Davide Libenzi wrote:
>>
>>> That was a zillions of years ago :) , but from a quick look at email
>>> thread, if you want to do it within xdiff,
On Thu, Apr 14, 2016 at 02:05:03PM -0700, Stefan Beller wrote:
> > Looking over the code, I agree that xdl_change_compact() is the place we
> > would want to put it. We'd probably tie it to a command-line option and
> > let people play around with it, and then consider making it the default
> > if
TODO(sbeller):
* describe the discussion on why this is better
* see if this can be tested?
Signed-off-by: Stefan Beller
---
xdiff/xdiffi.c | 39 +++
1 file changed, 39 insertions(+)
diff --git a/xdiff/xdiffi.c b/xdiff/xdiffi.c
index 2358a2d..24eb9a0 100644
-
On Thu, Apr 14, 2016 at 5:07 PM, Stefan Beller wrote:
> TODO(sbeller):
> * describe the discussion on why this is better
> * see if this can be tested?
>
Thanks for taking time to do this! It looks like a few things are
still missing, CRLF obviously, and making it a configuration option.
> Signe
On Thu, Apr 14, 2016 at 5:26 PM, Jacob Keller wrote:
> On Thu, Apr 14, 2016 at 5:07 PM, Stefan Beller wrote:
>> TODO(sbeller):
>> * describe the discussion on why this is better
>> * see if this can be tested?
>>
>
> Thanks for taking time to do this! It looks like a few things are
> still missin
On Thu, Apr 14, 2016 at 5:43 PM, Stefan Beller wrote:
> On Thu, Apr 14, 2016 at 5:26 PM, Jacob Keller wrote:
>> On Thu, Apr 14, 2016 at 5:07 PM, Stefan Beller wrote:
>>> TODO(sbeller):
>>> * describe the discussion on why this is better
>>> * see if this can be tested?
>>>
>>
>> Thanks for takin
Stefan Beller writes:
>
> +static int starts_with_emptyline(const char *recs)
> +{
> + return recs[0] == '\n'; /* CRLF not covered here */
> +}
> +
> +
That's "is-empty-line", not "starts-with" ;-)
> +
> + /*
> + * If a group can be moved back and forth, see if th
On Thu, Apr 14, 2016 at 7:09 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>>
>> +static int starts_with_emptyline(const char *recs)
>> +{
>> + return recs[0] == '\n'; /* CRLF not covered here */
>> +}
>> +
>> +
>
> That's "is-empty-line", not "starts-with" ;-)
heh, ok.
To understand t
From: Jacob Keller
I took up Stefan's patch, and modified it to resolve a couple issues. I
also tried to implement the suggestions from Junio's review, as well as
the suggestions I had. It appears to produce equivalent output as Jeff's
script. This version is still missing a few things, and it is
From: Jacob Keller
It is a common pattern in xdl_change_compact to check that hashes and
strings match. The resulting code to perform this change causes very
long lines and makes it hard to follow the intention. Introduce a helper
function xdl_hash_and_recmatch which performs both checks to incre
On Fri, Apr 15, 2016 at 9:51 AM, Jacob Keller wrote:
> From: Jacob Keller
>
> I took up Stefan's patch, and modified it to resolve a couple issues. I
> also tried to implement the suggestions from Junio's review, as well as
> the suggestions I had. It appears to produce equivalent output as Jeff'
On Fri, Apr 15, 2016 at 10:02 AM, Stefan Beller wrote:
>> @@ -458,11 +458,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo,
>> long flags) {
>> * the group.
>> */
>> while (ixs > 0 && xdl_hash_and_recmatch(recs, i
Stefan Beller writes:
> Actually we would only need to have the empty line count in the
> second loop as the first loop shifted it as much up(backwards) as
> possible, such that the hunk sits on one end of the movable
> range. The second loop would iterate over the whole range, so that
> would be
On Fri, Apr 15, 2016 at 10:27 AM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> Actually we would only need to have the empty line count in the
>> second loop as the first loop shifted it as much up(backwards) as
>> possible, such that the hunk sits on one end of the movable
>> range. The se
Stefan Beller writes:
> I think we need to be aggressive to find adjacent groups and only after
> that is done we should think about optimizing look&feel?
OK. I was just gauging to see if those involved in the codepath
thought things through, and apparently you did, so I'm happy ;-)
Thanks.
--
On Fri, Apr 15, 2016 at 10:02 AM, Stefan Beller wrote:
> On Fri, Apr 15, 2016 at 9:51 AM, Jacob Keller
> wrote:
>> From: Jacob Keller
>>
>> I took up Stefan's patch, and modified it to resolve a couple issues. I
>> also tried to implement the suggestions from Junio's review, as well as
>> the s
On Fri, Apr 15, 2016 at 10:10 AM, Stefan Beller wrote:
> On Fri, Apr 15, 2016 at 10:02 AM, Stefan Beller wrote:
>>> @@ -458,11 +458,11 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo,
>>> long flags) {
>>> * the group.
>>> */
>>>
On Fri, Apr 15, 2016 at 10:33 AM, Stefan Beller wrote:
> On Fri, Apr 15, 2016 at 10:27 AM, Junio C Hamano wrote:
>> Stefan Beller writes:
>>
>>> Actually we would only need to have the empty line count in the
>>> second loop as the first loop shifted it as much up(backwards) as
>>> possible, suc
From: Stefan Beller
In order to produce the smallest possible diff and combine several diff
hunks together, we implement a heuristic from GNU Diff which moves diff
hunks forward as far as possible when we find common context above and
below a diff hunk. This sometimes produces less readable diffs
Jacob Keller writes:
> From: Jacob Keller
>
> It is a common pattern in xdl_change_compact to check that hashes and
> strings match. The resulting code to perform this change causes very
> long lines and makes it hard to follow the intention. Introduce a helper
> function xdl_hash_and_recmatch w
On Fri, Apr 15, 2016 at 10:25 AM, Junio C Hamano wrote:
> Jacob Keller writes:
>
>> From: Jacob Keller
>>
>> It is a common pattern in xdl_change_compact to check that hashes and
>> strings match. The resulting code to perform this change causes very
>> long lines and makes it hard to follow the
From: Jacob Keller
It is a common pattern in xdl_change_compact to check that hashes and
strings match. The resulting code to perform this change causes very
long lines and makes it hard to follow the intention. Introduce a helper
function recs_match which performs both checks to increase
code re
From: Jacob Keller
It is a common pattern in xdl_change_compact to check that hashes and
strings match. The resulting code to perform this change causes very
long lines and makes it hard to follow the intention. Introduce a helper
function recs_match which performs both checks to increase
code re
On Fri, Apr 15, 2016 at 9:51 AM, Jacob Keller wrote:
> From: Stefan Beller
>
> In order to produce the smallest possible diff and combine several diff
> hunks together, we implement a heuristic from GNU Diff which moves diff
> hunks forward as far as possible when we find common context above and
Stefan Beller writes:
>> +diff.emptyLineHeuristic::
>
> I was looking at the TODO here and thought about the name:
> It should not encode the `emptyLine` into the config option as
> it is only one of many heuristics.
>
> It should be something like `diff.heuristic=lastEmptyLine`
> The we could ad
On Fri, Apr 15, 2016 at 12:57 PM, Stefan Beller wrote:
> I was looking at the TODO here and thought about the name:
> It should not encode the `emptyLine` into the config option as
> it is only one of many heuristics.
>
> It should be something like `diff.heuristic=lastEmptyLine`
> The we could ad
On Fri, Apr 15, 2016 at 1:06 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>>> +diff.emptyLineHeuristic::
>>
>> I was looking at the TODO here and thought about the name:
>> It should not encode the `emptyLine` into the config option as
>> it is only one of many heuristics.
>>
>> It should
Jacob Keller writes:
> On Fri, Apr 15, 2016 at 1:06 PM, Junio C Hamano wrote:
>
>> I actually do not think these knobs should exist when the code is
>> mature enough to be shipped to the end users.
>>
>> Use "diff.compactionHeuristics = " as an opaque set of bits to
>> help the developers while
On Fri, Apr 15, 2016 at 1:30 PM, Junio C Hamano wrote:
> Jacob Keller writes:
>
>> On Fri, Apr 15, 2016 at 1:06 PM, Junio C Hamano wrote:
>>
>>> I actually do not think these knobs should exist when the code is
>>> mature enough to be shipped to the end users.
>>>
>>> Use "diff.compactionHeurist
On Fri, Apr 15, 2016 at 2:15 PM, Jacob Keller wrote:
> On Fri, Apr 15, 2016 at 1:30 PM, Junio C Hamano wrote:
>> Jacob Keller writes:
>>
>>> On Fri, Apr 15, 2016 at 1:06 PM, Junio C Hamano wrote:
>>>
I actually do not think these knobs should exist when the code is
mature enough to be
Jacob Keller writes:
>>> What you have is a pure developer support; aim to come up with "good
>>> enough" way, giving developers an easier way to experiment with, and
>>> remove it before the feature is shipped to the end user.
>
> What are your thoughts on adding this do the gitattributes diff
>
From: Jacob Keller
Second version of my series with a few more minor fixups. I left the
diff command line and configuration option alone for now, suspecting
that long term we either (a) remove it or (b) use a gitattribute, so
there is no reason to bikeshed the name or its contents right now.
TOD
From: Stefan Beller
In order to produce the smallest possible diff and combine several diff
hunks together, we implement a heuristic from GNU Diff which moves diff
hunks forward as far as possible when we find common context above and
below a diff hunk. This sometimes produces less readable diffs
On Fri, Apr 15, 2016 at 2:44 PM, Junio C Hamano wrote:
> Jacob Keller writes:
>
What you have is a pure developer support; aim to come up with "good
enough" way, giving developers an easier way to experiment with, and
remove it before the feature is shipped to the end user.
>>
>> W
On Fri, Apr 15, 2016 at 02:56:21PM -0700, Jacob Keller wrote:
> @@ -470,8 +477,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo,
> long flags) {
>* the line next of the current change group, shift
> forward
>* the group.
>
On Fri, Apr 15, 2016 at 3:46 PM, Jeff King wrote:
> On Fri, Apr 15, 2016 at 02:56:21PM -0700, Jacob Keller wrote:
>
>> @@ -470,8 +477,9 @@ int xdl_change_compact(xdfile_t *xdf, xdfile_t *xdfo,
>> long flags) {
>>* the line next of the current change group, shift
>> forwar
From: Jacob Keller
Third version of my series with a few more minor fixups. I left the
diff command line and configuration option alone for now, suspecting
that long term we either (a) remove it or (b) use a gitattribute, so
there is no reason to bikeshed the name or its contents right now.
TODO
From: Stefan Beller
In order to produce the smallest possible diff and combine several diff
hunks together, we implement a heuristic from GNU Diff which moves diff
hunks forward as far as possible when we find common context above and
below a diff hunk. This sometimes produces less readable diffs
This is a version based on Jacobs v2, with the same fixes as in his v3
(hopefully),
changing the heuristic, such that CRLF confusion might be gone.
TODO:
* add some tests
* think about whether we need a git attribute or not (I did some
thinking, and if we do need to configure this at all, this
58 matches
Mail list logo
