On Mon, May 2, 2016 at 11:02 AM, Jeff King wrote:
> On Mon, May 02, 2016 at 10:40:28AM -0700, Junio C Hamano wrote:
>
>> "Keller, Jacob E" writes:
>>
>> > True. I think the chances that it needs such a thing are quite minor,
>> > and if an undocumented knob gets exposed it would have to become
>>
On Mon, May 02, 2016 at 10:40:28AM -0700, Junio C Hamano wrote:
> "Keller, Jacob E" writes:
>
> > True. I think the chances that it needs such a thing are quite minor,
> > and if an undocumented knob gets exposed it would have to become
> > documented and maintained, so I'd prefer to avoid it. G
On Mon, May 2, 2016 at 10:40 AM, Junio C Hamano wrote:
> "Keller, Jacob E" writes:
>
>> True. I think the chances that it needs such a thing are quite minor,
>> and if an undocumented knob gets exposed it would have to become
>> documented and maintained, so I'd prefer to avoid it. Given that the
"Keller, Jacob E" writes:
> True. I think the chances that it needs such a thing are quite minor,
> and if an undocumented knob gets exposed it would have to become
> documented and maintained, so I'd prefer to avoid it. Given that the
> risk is pretty small I think that's ok.
OK, then let's do
On Fri, Apr 29, 2016 at 03:35:54PM -0700, Stefan Beller wrote:
> > -- >8 --
> > diff: enable "compaction heuristics" and lose experimentation knob
> >
> > It seems that the new "find a good hunk boundary by locating a blank
> > line" heuristics gives much more pleasant result without much
> > noti
On Fri, 2016-04-29 at 15:44 -0700, Stefan Beller wrote:
> >
> > Currently it's an "opt in" knob, so this doesn't make sense to me.
> +static int diff_compaction_heuristic = 1;
>
Oops didn't know we'd made it default at some point. (all my versions
had it disabled by default)
> It's rather an op
> Currently it's an "opt in" knob, so this doesn't make sense to me.
+static int diff_compaction_heuristic = 1;
It's rather an opt-out knob going by the current
origin/jk/diff-compact-heuristic
> If
> we remove the entire knob as is, we can always (fairly easily) add it
> back. I would keep the
On Fri, 2016-04-29 at 15:35 -0700, Stefan Beller wrote:
> On Fri, Apr 29, 2016 at 3:18 PM, Junio C Hamano
> wrote:
> >
> > Jacob Keller writes:
> >
> > >
> > > On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano > > m> wrote:
> > > >
> > > > Jeff King writes:
> > > >
> > > > >
> > > > > ... H
On Fri, Apr 29, 2016 at 3:18 PM, Junio C Hamano wrote:
> Jacob Keller writes:
>
>> On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano wrote:
>>> Jeff King writes:
>>>
... Having the two directly next to each other reads
better to me. This is a pretty unusual diff, though, in that it did
Jacob Keller writes:
> On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano wrote:
>> Jeff King writes:
>>
>>> ... Having the two directly next to each other reads
>>> better to me. This is a pretty unusual diff, though, in that it did
>>> change the surrounding whitespace (and if you look further i
On Fri, Apr 29, 2016 at 1:29 PM, Junio C Hamano wrote:
> Jeff King writes:
>
>> ... Having the two directly next to each other reads
>> better to me. This is a pretty unusual diff, though, in that it did
>> change the surrounding whitespace (and if you look further in the diff,
>> the identical c
Jeff King writes:
> ... Having the two directly next to each other reads
> better to me. This is a pretty unusual diff, though, in that it did
> change the surrounding whitespace (and if you look further in the diff,
> the identical change is made elsewhere _without_ touching the
> whitespace). S
On Wed, Apr 20, 2016 at 09:09:53AM -0700, Junio C Hamano wrote:
> "Michael S. Tsirkin" writes:
>
> > FWIW IIRC what that commit is about is ability to reorder the chunks in
> > a patch without changing patch-id. Not about keeping id stable across
> > git revisions.
>
> OK, but "reorder the chun
"Michael S. Tsirkin" writes:
> FWIW IIRC what that commit is about is ability to reorder the chunks in
> a patch without changing patch-id. Not about keeping id stable across
> git revisions.
OK, but "reorder the chunks" is not meant to stay to be the _ONLY_
purpose for an option whose name is a
On Tue, Apr 19, 2016 at 04:07:35PM -0700, Junio C Hamano wrote:
> Jacob Keller writes:
>
> > On Tue, Apr 19, 2016 at 10:06 AM, Jeff King wrote:
> >> On Tue, Apr 19, 2016 at 08:17:38AM -0700, Stefan Beller wrote:
> >>
> >>> On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
> >>>
> >>> > I guess
Jeff King writes:
> I mean that if you save any old patch-ids from "git patch-id", they
> won't match up when compared with new versions of git. We can probably
> ignore it, though. This isn't the first time that patch-ids might have
> changed, and I think the advice is already that one should no
On Tue, Apr 19, 2016 at 9:18 PM, Jeff King wrote:
> [your original probably didn't make it to the list because of its 5MB
> attachment; the list has a 100K limit; I'll try to quote liberally]
>
> On Tue, Apr 19, 2016 at 04:17:50PM -0700, Jacob Keller wrote:
>
>> I ran this version of the patch ag
On Wed, Apr 20, 2016 at 12:18:27AM -0400, Jeff King wrote:
> My earlier tests with the perl script were all done with "git log -p",
> which will not show anything at all for merges (and my script wouldn't
> know how to deal with combined diffs anyway). But I think this new patch
> _will_ kick in f
[your original probably didn't make it to the list because of its 5MB
attachment; the list has a 100K limit; I'll try to quote liberally]
On Tue, Apr 19, 2016 at 04:17:50PM -0700, Jacob Keller wrote:
> I ran this version of the patch against the entire Linux kernel
> history, as I figured this h
Jacob Keller writes:
> On Tue, Apr 19, 2016 at 10:06 AM, Jeff King wrote:
>> On Tue, Apr 19, 2016 at 08:17:38AM -0700, Stefan Beller wrote:
>>
>>> On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
>>>
>>> > I guess this will invalidate old patch-ids, but there's not much to be
>>> > done about
On Tue, Apr 19, 2016 at 10:06 AM, Jeff King wrote:
> On Tue, Apr 19, 2016 at 08:17:38AM -0700, Stefan Beller wrote:
>
>> On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
>>
>> > I guess this will invalidate old patch-ids, but there's not much to be
>> > done about that.
>>
>> What do you mean b
On Tue, Apr 19, 2016 at 08:17:38AM -0700, Stefan Beller wrote:
> On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
>
> > I guess this will invalidate old patch-ids, but there's not much to be
> > done about that.
>
> What do you mean by that? (What consequences do you imagine?)
> I think diffs
Jeff King writes:
> I guess this will invalidate old patch-ids, but there's not much to be
> done about that.
If we really cared, we could disable this (and any future) change to
the compaction logic to "patch-id --[un]stable" option.
I am not sure if it is worth the effort, though ;-)
--
To un
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 when
writing C, Shell
On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
> I guess this will invalidate old patch-ids, but there's not much to be
> done about that.
What do you mean by that? (What consequences do you imagine?)
I think diffs with any kind of heuristic can still be applied, no?
Thanks,
Stefan
>
> -Pe
On Tue, Apr 19, 2016 at 12:00 AM, Jeff King wrote:
> On Mon, Apr 18, 2016 at 11:47:52PM -0700, Stefan Beller wrote:
>
>> I am convinced the better way to do it is like this:
>>
>> Calculate the entropy for each line and take the last line with the
>> lowest entropy as the last line of the
On Mon, Apr 18, 2016 at 11:47:52PM -0700, Stefan Beller wrote:
> I am convinced the better way to do it is like this:
>
> Calculate the entropy for each line and take the last line with the
> lowest entropy as the last line of the hunk.
I'll be curious to see the results, but I think som
On Mon, Apr 18, 2016 at 10:03 PM, Jeff King wrote:
> On Mon, Apr 18, 2016 at 02:12:30PM -0700, Stefan Beller wrote:
>
>> +
>> + /*
>> + * If a group can be moved back and forth, see if there is an
>> + * blank line in the moving space. If there is a blank line
On Mon, Apr 18, 2016 at 02:12:30PM -0700, Stefan Beller wrote:
> +
> + /*
> + * If a group can be moved back and forth, see if there is an
> + * blank line in the moving space. If there is a blank line,
> + * make sure the last blank line is the e
Jacob Keller writes:
> Thanks Stephan and Junio, this looks pretty good. I think before it's
> merged we'd probably want to implement some sort of attributes which
> allows per-path configuration, incase it needs to be configured at
> all.
My take on it is that we'd want to make sure that the sh
On Mon, Apr 18, 2016 at 2:12 PM, Stefan Beller wrote:
> 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. Th
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 when
writing C, Shell
On Mon, Apr 18, 2016 at 12:22 PM, Junio C Hamano wrote:
> Jacob Keller writes:
>
>> I think we're going to make use of xdl_blankline instead of this or
>> our own "is_emptyline"
>
> OK, so perhaps either of you two can do a final version people can
> start having fun with?
Junios proposal seems
Jacob Keller writes:
> I think we're going to make use of xdl_blankline instead of this or
> our own "is_emptyline"
OK, so perhaps either of you two can do a final version people can
start having fun with?
By the way, I really do not want to see something this low-level to
be end-user tweakable
On Fri, Apr 15, 2016 at 5:49 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> +static int line_length(const char *recs)
>> +{
>> + char *s = strchr(recs, '\n');
>> + return s ? s - recs : strlen(recs);
>> +}
>
> It seems that you guys are discarding this "number of bytes on a
> line
On Fri, Apr 15, 2016 at 5:49 PM, Junio C Hamano wrote:
> Stefan Beller writes:
>
>> +static int line_length(const char *recs)
>> +{
>> + char *s = strchr(recs, '\n');
>> + return s ? s - recs : strlen(recs);
>> +}
>
> It seems that you guys are discarding this "number of bytes on a
> line
Stefan Beller writes:
> +static int line_length(const char *recs)
> +{
> + char *s = strchr(recs, '\n');
> + return s ? s - recs : strlen(recs);
> +}
It seems that you guys are discarding this "number of bytes on a
line, no matter what these bytes are" idea, so this may be moot, but
is t
On Fri, Apr 15, 2016 at 4:32 PM, Jacob Keller wrote:
> On Fri, Apr 15, 2016 at 4:05 PM, Jacob Keller wrote:
>> There's a few places that will need cleaning up (comments and such)
>> that mention empty line still, but that's not surprising. I am going
>> to test this for a bit on my local repos, a
On Fri, Apr 15, 2016 at 4:05 PM, Jacob Keller wrote:
> There's a few places that will need cleaning up (comments and such)
> that mention empty line still, but that's not surprising. I am going
> to test this for a bit on my local repos, and see if it makes any
> difference to the old heuristic as
On Fri, Apr 15, 2016 at 4:01 PM, Stefan Beller wrote:
> 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. Th
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 when
writing C, Shell
41 matches
Mail list logo
