From: Junio C Hamano <gitster@pobox.com> To: Jeff King <peff@peff.net> Cc: "Martin Ågren" <martin.agren@gmail.com>, git@vger.kernel.org, "Emily Shaffer" <emilyshaffer@google.com> Subject: Re: [PATCH 3/4] grep: simplify color setup Date: Sat, 21 Nov 2020 14:46:14 -0800 Message-ID: <xmqq4klis4zt.fsf@gitster.c.googlers.com> (raw) In-Reply-To: <20201121202310.GA972561@coredump.intra.peff.net> (Jeff King's message of "Sat, 21 Nov 2020 15:23:10 -0500") Jeff King <peff@peff.net> writes: > On Sat, Nov 21, 2020 at 07:31:09PM +0100, Martin Ågren wrote: > >> The previous commit left us with only one user of the one-line wrapper >> `color_set()`. We could inline it, but note how we're `xsnprintf()`-ing >> all the entries in one array into another array of the same type. We >> might as well just `memcpy()` everything into place. >> >> Signed-off-by: Martin Ågren <martin.agren@gmail.com> >> --- >> Cc-ing Peff, who initially introduced this helper. After having inlined >> the function into the for loop, it seemed better to just copy the whole >> array. Happy to hear arguments against. > > No, this is way better than the existing code. I introduced it to get > away from strcpy(), but this is better still. But... Yes, the copy in this patch looks eminently sensible. >> Come to think of it, I suppose we could copy the whole struct and not >> just the color array. Hmmm... > > Yes, this seems even better. If our goal is just to start our new > grep_opt the same as grep_defaults, then a single-line struct copy > (whether through assignment or memcpy) is even clearer and more > maintainable. ... until such a time when typeof(grep_defaults) gains a field with a non-const pointer value that we'd rather not to share amongst instances of the type, at which point it no longer is clear win from maintainability's point of view.
next prev parent reply other threads:[~2020-11-21 22:48 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-21 18:31 [PATCH 0/4] grep: retire `init_grep_defaults()` Martin Ågren 2020-11-21 18:31 ` [PATCH 1/4] grep: don't set up a "default" repo for grep Martin Ågren 2020-11-21 18:31 ` [PATCH 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren 2020-11-21 18:31 ` [PATCH 3/4] grep: simplify color setup Martin Ågren 2020-11-21 20:23 ` Jeff King 2020-11-21 20:52 ` Martin Ågren 2020-11-21 22:46 ` Junio C Hamano [this message] 2020-11-24 6:54 ` Jeff King 2020-11-21 18:31 ` [PATCH 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren 2020-11-23 11:03 ` [PATCH 0/4] grep: retire `init_grep_defaults()` Johannes Schindelin 2020-11-24 21:04 ` [PATCH v2 0/4] grep: simplify "grep defaults" handling Martin Ågren 2020-11-24 21:04 ` [PATCH v2 1/4] grep: don't set up a "default" repo for grep Martin Ågren 2020-11-24 21:04 ` [PATCH v2 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren 2020-11-24 21:04 ` [PATCH v2 3/4] grep: copy struct in one fell swoop Martin Ågren 2020-11-24 22:34 ` Junio C Hamano 2020-11-25 6:25 ` Martin Ågren 2020-11-25 7:53 ` Junio C Hamano 2020-11-26 20:25 ` Martin Ågren 2020-11-24 21:04 ` [PATCH v2 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren 2020-11-25 9:27 ` [PATCH v2 0/4] grep: simplify "grep defaults" handling Ævar Arnfjörð Bjarmason 2020-11-29 19:52 ` [PATCH v3 " Martin Ågren 2020-11-29 19:52 ` [PATCH v3 1/4] grep: don't set up a "default" repo for grep Martin Ågren 2020-11-29 19:52 ` [PATCH v3 2/4] grep: use designated initializers for `grep_defaults` Martin Ågren 2020-11-29 19:52 ` [PATCH v3 3/4] grep: copy struct in one fell swoop Martin Ågren 2020-11-29 19:52 ` [PATCH v3 4/4] MyFirstObjectWalk: drop `init_walken_defaults()` Martin Ågren 2020-12-01 4:46 ` [PATCH v3 0/4] grep: simplify "grep defaults" handling Junio C Hamano
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=xmqq4klis4zt.fsf@gitster.c.googlers.com \ --to=gitster@pobox.com \ --cc=emilyshaffer@google.com \ --cc=git@vger.kernel.org \ --cc=martin.agren@gmail.com \ --cc=peff@peff.net \ /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
git@vger.kernel.org list mirror (unofficial, one of many) This inbox may be cloned and mirrored by anyone: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V1 git git/ https://public-inbox.org/git \ git@vger.kernel.org public-inbox-index git Example config snippet for mirrors. Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.io/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ code repositories for the project(s) associated with this inbox: https://80x24.org/mirrors/git.git AGPL code for this site: git clone https://public-inbox.org/public-inbox.git