Re: [PATCH 3/7] diffcore-rename: rename num_create to num_targets

2 messages, 2 authors, 2020-12-10 · open the first message on its own page

Re: [PATCH 3/7] diffcore-rename: rename num_create to num_targets

From: Junio C Hamano <hidden>
Date: 2020-12-10 02:21:42

"Elijah Newren via GitGitGadget" [off-list ref] writes:
From: Elijah Newren <redacted>

Files added since the base commit serve as targets for rename detection.
While it is true that added files can be thought of as being "created"
when they are added IF they have no pairing file that they were renamed
from, and it is true we start out not knowing what the pairings are, it
seems a little odd to think in terms of "file creation" when we are
looking for "file renames".  Rename the variable to avoid this minor
point of confusion.
This is probably subjective.  

I've always viewed the rename detection as first collecting a set of
deleted paths and a set of created paths, and then trying to find a
good mapping from the former into the latter, so I find num_create a
lot more intuitive than num_targets.

But the remaining elements in the latter set are counted in the
counter "rename_dst_nr", so we clearly are OK to call the elements
of the latter set "the destination" (of a rename), which contrasts
very well with "the source" (of a rename), which is how the deleted
paths are counted with rename_src_nr.

When doing -C and -C -C, the "source" set has not just deleted but
also the preimage of the modified paths, so "source" is a more
appropriate name than "delete".  From that point of view,
"destination" is a more appropriate name for "create" because it
contrasts well with "source".

You silently renamed num_create to num_targets in 1/7 without
justification while adding num_sources.  Perhaps we should go back
to that step and use num_destinations to match?  The result would be
using words <dst, src> that pair with each other much better than
introducing "target" to an existing mix of <create==dst, src> to
make it <target==dst, src>.

Re: [PATCH 3/7] diffcore-rename: rename num_create to num_targets

From: Elijah Newren <hidden>
Date: 2020-12-10 02:26:35

On Wed, Dec 9, 2020 at 6:20 PM Junio C Hamano [off-list ref] wrote:
"Elijah Newren via GitGitGadget" [off-list ref] writes:
quoted
From: Elijah Newren <redacted>

Files added since the base commit serve as targets for rename detection.
While it is true that added files can be thought of as being "created"
when they are added IF they have no pairing file that they were renamed
from, and it is true we start out not knowing what the pairings are, it
seems a little odd to think in terms of "file creation" when we are
looking for "file renames".  Rename the variable to avoid this minor
point of confusion.
This is probably subjective.

I've always viewed the rename detection as first collecting a set of
deleted paths and a set of created paths, and then trying to find a
good mapping from the former into the latter, so I find num_create a
lot more intuitive than num_targets.

But the remaining elements in the latter set are counted in the
counter "rename_dst_nr", so we clearly are OK to call the elements
of the latter set "the destination" (of a rename), which contrasts
very well with "the source" (of a rename), which is how the deleted
paths are counted with rename_src_nr.

When doing -C and -C -C, the "source" set has not just deleted but
also the preimage of the modified paths, so "source" is a more
appropriate name than "delete".  From that point of view,
"destination" is a more appropriate name for "create" because it
contrasts well with "source".

You silently renamed num_create to num_targets in 1/7 without
justification while adding num_sources.  Perhaps we should go back
to that step and use num_destinations to match?  The result would be
using words <dst, src> that pair with each other much better than
introducing "target" to an existing mix of <create==dst, src> to
make it <target==dst, src>.
Ooh, I like it.  num_destinations it is.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help