From: Jeff King <email@example.com> To: Thomas Guyot-Sionnest <firstname.lastname@example.org> Cc: Taylor Blau <email@example.com>, firstname.lastname@example.org, Thomas Guyot-Sionnest <email@example.com> Subject: Re: [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat Date: Fri, 18 Sep 2020 13:37:39 -0400 [thread overview] Message-ID: <20200918173739.GE183026@coredump.intra.peff.net> (raw) In-Reply-To: <CALqVohfQZu=itUyfU7nubJpgBETh2q7W1TVx=c2E32ey2cFZkA@mail.gmail.com> On Fri, Sep 18, 2020 at 11:10:45AM -0400, Thomas Guyot-Sionnest wrote: > > So, my last point is the most important of the three. I'd expect > > something more along the lines of: > > > > 1. diff_fill_oid_info resolve the link to the pipe, and > > 2. index_path handles the resolved fd. > > > > ...but it looks like that is already what it's doing? I'm confused why > > this doesn't work as-is. > > So the idea is to checksum the data and write a valid oid. I'll see if > I can figure that out. Thanks for the hint though else I would likely > have gone with a buffer and memcmp. Your solution seems cleaner, and > there is a few other uses of oideq's that look dubious at best with > the case of null oids / buffered data so it's definitely a better > approach. You're generally better off not to compute the oid just to compare two buffers: - a byte-by-byte comparison can quit early as soon as it sees a difference, whereas computing two hashes has to cover each byte - even in the worst case that the byte comparison has to go all the way to the end, it's way faster than computing a sha1 So generally in the diff code we compare oids if we got them for free (from the index, or from a tree), but otherwise it's OK to have filespecs without the oid_valid flag set, and compare them byte-wise when necessary. And there something like: if (one->size == two->size && !memcmp(one->data, two->data, one->size)) is what you'd want. Note that filespecs may not have their data or size loaded yet, though. Looking at that part of builtin_diffstat(), I'm pretty sure that is possible here (see how later code calls diff_populate_filespec() to make sure it has data). OTOH, I guess if they're from stdin we'd always have the data (since we'd have no oid to load from), so it might be OK under that conditional. -Peff
next prev parent reply other threads:[~2020-09-18 17:37 UTC|newest] Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-09-18 11:32 Allow passing pipes to diff --no-index + bugfix Thomas Guyot-Sionnest 2020-09-18 11:32 ` [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat Thomas Guyot-Sionnest 2020-09-18 14:46 ` Taylor Blau 2020-09-18 15:10 ` Thomas Guyot-Sionnest 2020-09-18 17:37 ` Jeff King [this message] 2020-09-18 18:00 ` Thomas Guyot-Sionnest 2020-09-20 4:53 ` Thomas Guyot 2020-09-18 17:27 ` Jeff King 2020-09-18 17:52 ` Thomas Guyot-Sionnest 2020-09-18 18:06 ` Junio C Hamano 2020-09-23 19:16 ` Johannes Schindelin 2020-09-23 19:23 ` Junio C Hamano 2020-09-23 20:44 ` Johannes Schindelin 2020-09-24 4:49 ` Thomas Guyot 2020-09-24 5:24 ` [PATCH v3] " Thomas Guyot-Sionnest 2020-09-24 7:41 ` [PATCH v4] " Thomas Guyot-Sionnest 2020-09-24 6:40 ` [PATCH 1/2] " Junio C Hamano 2020-09-24 7:13 ` Thomas Guyot 2020-09-24 17:19 ` Junio C Hamano 2020-09-24 17:38 ` Junio C Hamano 2020-09-23 15:05 ` Johannes Schindelin 2020-09-20 13:09 ` [PATCH v2] " Thomas Guyot-Sionnest 2020-09-20 15:39 ` Taylor Blau 2020-09-20 16:38 ` Thomas Guyot 2020-09-20 19:11 ` Junio C Hamano 2020-09-20 20:08 ` Junio C Hamano 2020-09-20 20:36 ` Junio C Hamano 2020-09-20 22:15 ` Junio C Hamano 2020-09-21 19:26 ` Jeff King 2020-09-21 21:51 ` Junio C Hamano 2020-09-21 22:20 ` Jeff King 2020-09-21 22:37 ` Junio C Hamano 2020-09-18 11:32 ` [PATCH 2/2] Allow passing pipes for input pipes to diff --no-index Thomas Guyot-Sionnest 2020-09-18 14:36 ` Taylor Blau 2020-09-18 16:34 ` Thomas Guyot-Sionnest 2020-09-18 17:19 ` Jeff King 2020-09-18 17:21 ` Jeff King 2020-09-18 17:39 ` Thomas Guyot-Sionnest 2020-09-18 17:48 ` Junio C Hamano 2020-09-18 18:02 ` Jeff King 2020-09-20 12:54 ` Thomas Guyot 2020-09-21 19:31 ` Jeff King 2020-09-21 20:14 ` Junio C Hamano 2020-09-18 17:58 ` Taylor Blau 2020-09-18 18:05 ` Jeff King 2020-09-18 17:20 ` Jeff King 2020-09-18 18:00 ` Taylor Blau 2020-09-18 21:56 ` brian m. carlson 2020-09-18 17:51 ` Allow passing pipes to diff --no-index + bugfix Junio C Hamano 2020-09-18 18:24 ` Thomas Guyot-Sionnest
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=20200918173739.GE183026@coredump.intra.peff.net \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --subject='Re: [PATCH 1/2] diff: Fix modified lines stats with --stat and --numstat' \ /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
Code repositories for project(s) associated with this 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).