From: Johannes Schindelin <Johannes.Schindelin@gmx.de>
To: Alex Vandiver <alexmv@dropbox.com>
Cc: git@vger.kernel.org, Ben Peart <peartben@gmail.com>
Subject: Re: [PATCH 6/6] fsmonitor: Use fsmonitor data in `git diff`
Date: Thu, 4 Jan 2018 23:46:27 +0100 (STD) [thread overview]
Message-ID: <alpine.DEB.2.21.1.1801042335130.32@MININT-6BKU6QN.europe.corp.microsoft.com> (raw)
In-Reply-To: <121828fc14bc6f3096d16005feffb58bf68f070a.1514948078.git.alexmv@dropbox.com>
Hi Alex,
On Tue, 2 Jan 2018, Alex Vandiver wrote:
> diff --git a/diff-lib.c b/diff-lib.c
> index 8104603a3..13ff00d81 100644
> --- a/diff-lib.c
> +++ b/diff-lib.c
> @@ -95,6 +95,9 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
>
> diff_set_mnemonic_prefix(&revs->diffopt, "i/", "w/");
>
> + if (!(option & DIFF_SKIP_FSMONITOR))
> + refresh_fsmonitor(&the_index);
> +
> if (diff_unmerged_stage < 0)
> diff_unmerged_stage = 2;
I read over this hunk five times, and only now am I able to wrap my head
around this: if we do *not* want to skip the fsmonitor data, we refresh
the fsmonitor data in the index.
That feels a bit like an unneeded double negation. Speaking for myself, I
would prefore `DIFF_IGNORE_FSMONITOR` instead, it would feel less like a
double negation then. But I am not a native speaker, so I might be wrong.
> + if (ce->ce_flags & CE_FSMONITOR_VALID && !(option & DIFF_SKIP_FSMONITOR))
> + continue;
Since we do expect this to be called without the DIFF_SKIP_FSMONITOR flag,
I guess it makes sense to order it this way.
I still have troubles to understand why we ignore the fsmonitor data with
`git add`, though... we want to add only modified files, right? I thought
that the fsmonitor data could help performance exactly there (I am
thinking of a certain insanely large code base where a developer might
want to change only one or maybe 3 files out of an entire machine workshop
of files, and with fsmonitor it should be a really fast operation because
it should ignore all but those few files, right?)... Could you maybe try
to help me understand that better?
Thanks,
Johannes
next prev parent reply other threads:[~2018-01-04 22:46 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-01-03 3:04 [PATCH v2 0/6] Minor fsmonitor bugfixes, use with `git diff` Alex Vandiver
2018-01-03 3:04 ` [PATCH 1/6] Fix comments to agree with argument name Alex Vandiver
2018-01-03 3:04 ` [PATCH 2/6] fsmonitor: Stop inline'ing mark_fsmonitor_valid / _invalid Alex Vandiver
2018-01-04 22:27 ` Johannes Schindelin
2018-01-08 20:27 ` Ben Peart
2018-01-03 3:04 ` [PATCH 3/6] fsmonitor: Update helper tool, now that flags are filled later Alex Vandiver
2018-01-04 23:03 ` Johannes Schindelin
2018-01-03 3:04 ` [PATCH 4/6] fsmonitor: Make output of test-dump-fsmonitor more concise Alex Vandiver
2018-01-04 22:33 ` Johannes Schindelin
2018-01-08 20:33 ` Ben Peart
2018-01-03 3:04 ` [PATCH 5/6] fsmonitor: Remove debugging lines from t/t7519-status-fsmonitor.sh Alex Vandiver
2018-01-03 3:04 ` [PATCH 6/6] fsmonitor: Use fsmonitor data in `git diff` Alex Vandiver
2018-01-04 22:46 ` Johannes Schindelin [this message]
2018-01-05 22:22 ` Junio C Hamano
2018-01-08 20:58 ` Ben Peart
-- strict thread matches above, loose matches on Subject: below --
2017-12-19 0:28 [PATCH 0/6] Minor fsmonitor bugfixes, use with " Alex Vandiver
2017-12-19 0:28 ` [PATCH 1/6] Fix comments to agree with argument name Alex Vandiver
2017-12-19 0:28 ` [PATCH 6/6] fsmonitor: Use fsmonitor data in `git diff` Alex Vandiver
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
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=alpine.DEB.2.21.1.1801042335130.32@MININT-6BKU6QN.europe.corp.microsoft.com \
--to=johannes.schindelin@gmx.de \
--cc=alexmv@dropbox.com \
--cc=git@vger.kernel.org \
--cc=peartben@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.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
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 read-only IMAP folder(s) and NNTP newsgroup(s).