git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, Elijah Newren <newren@gmail.com>
Subject: Re: [PATCH 3/7] diffcore-rename: rename num_create to num_targets
Date: Wed, 09 Dec 2020 18:20:52 -0800	[thread overview]
Message-ID: <xmqqpn3ixuyj.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <30381addc5ca9f2b5299835020716291c7fe8d68.1607223276.git.gitgitgadget@gmail.com> (Elijah Newren via GitGitGadget's message of "Sun, 06 Dec 2020 02:54:32 +0000")

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Elijah Newren <newren@gmail.com>
>
> 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>.


  reply	other threads:[~2020-12-10  2:21 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-06  2:54 [PATCH 0/7] diffcore-rename improvements Elijah Newren via GitGitGadget
2020-12-06  2:54 ` [PATCH 1/7] diffcore-rename: avoid usage of global in too_many_rename_candidates() Elijah Newren via GitGitGadget
2020-12-09 22:06   ` Taylor Blau
2020-12-06  2:54 ` [PATCH 2/7] diffcore-rename: remove unnecessary if-clause Elijah Newren via GitGitGadget
2020-12-09 22:10   ` Taylor Blau
2020-12-10  0:32     ` Elijah Newren
2020-12-10  2:03     ` Junio C Hamano
2020-12-10  2:17       ` Elijah Newren
2020-12-10  6:56         ` Junio C Hamano
2020-12-06  2:54 ` [PATCH 3/7] diffcore-rename: rename num_create to num_targets Elijah Newren via GitGitGadget
2020-12-10  2:20   ` Junio C Hamano [this message]
2020-12-10  2:25     ` Elijah Newren
2020-12-06  2:54 ` [PATCH 4/7] diffcore-rename: change a few comments to use 'add' instead of 'create' Elijah Newren via GitGitGadget
2020-12-10  2:29   ` Junio C Hamano
2020-12-06  2:54 ` [PATCH 5/7] diffcore-rename: reduce jumpiness in progress counters Elijah Newren via GitGitGadget
2020-12-09 22:24   ` Taylor Blau
2020-12-10  2:36   ` Junio C Hamano
2020-12-06  2:54 ` [PATCH 6/7] diffcore-rename: simplify and accelerate register_rename_src() Elijah Newren via GitGitGadget
2020-12-09 22:40   ` Taylor Blau
2020-12-10  0:25     ` Elijah Newren
2020-12-10  0:41       ` Taylor Blau
2020-12-10  2:51   ` Junio C Hamano
2020-12-06  2:54 ` [PATCH 7/7] Accelerate rename_dst setup Elijah Newren via GitGitGadget
2020-12-09 23:01   ` Taylor Blau
2020-12-10  0:57     ` Elijah Newren
2020-12-10  1:43       ` Junio C Hamano
2020-12-06  3:01 ` [PATCH 0/7] diffcore-rename improvements Elijah Newren
2020-12-11  9:08 ` [PATCH v2 0/9] " Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 1/9] diffcore-rename: rename num_create to num_destinations Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 2/9] diffcore-rename: avoid usage of global in too_many_rename_candidates() Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 3/9] diffcore-rename: simplify limit check Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 4/9] diffcore-rename: reduce jumpiness in progress counters Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 5/9] t4058: add more tests and documentation for duplicate tree entry handling Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 6/9] t4058: explore duplicate tree entry handling in a bit more detail Elijah Newren via GitGitGadget
2021-04-21 12:29     ` Ævar Arnfjörð Bjarmason
2021-04-21 17:38       ` Elijah Newren
2020-12-11  9:08   ` [PATCH v2 7/9] diffcore-rename: simplify and accelerate register_rename_src() Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 8/9] diffcore-rename: accelerate rename_dst setup Elijah Newren via GitGitGadget
2020-12-11  9:08   ` [PATCH v2 9/9] diffcore-rename: remove unneccessary duplicate entry checks Elijah Newren via GitGitGadget
2020-12-29  8:31     ` Christian Couder
2020-12-29 18:09       ` Elijah Newren
2020-12-29 20:05   ` [PATCH v3 0/9] diffcore-rename improvements Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 1/9] diffcore-rename: rename num_create to num_destinations Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 2/9] diffcore-rename: avoid usage of global in too_many_rename_candidates() Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 3/9] diffcore-rename: simplify limit check Elijah Newren via GitGitGadget
2021-11-09 21:14       ` Başar Uğur
2021-11-10 20:06         ` Elijah Newren
2021-11-11  9:02           ` Başar Uğur
2021-11-11 16:19             ` Elijah Newren
2020-12-29 20:05     ` [PATCH v3 4/9] diffcore-rename: reduce jumpiness in progress counters Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 5/9] t4058: add more tests and documentation for duplicate tree entry handling Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 6/9] t4058: explore duplicate tree entry handling in a bit more detail Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 7/9] diffcore-rename: simplify and accelerate register_rename_src() Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 8/9] diffcore-rename: accelerate rename_dst setup Elijah Newren via GitGitGadget
2020-12-29 20:05     ` [PATCH v3 9/9] diffcore-rename: remove unnecessary duplicate entry checks Elijah Newren via GitGitGadget

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqpn3ixuyj.fsf@gitster.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=newren@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).