From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-ASN: AS53758 23.128.96.0/24 X-Spam-Status: No, score=-4.5 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_LOW,SPF_HELO_PASS,SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 818401F5AE for ; Thu, 20 May 2021 23:54:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234498AbhETXzU (ORCPT ); Thu, 20 May 2021 19:55:20 -0400 Received: from pb-smtp21.pobox.com ([173.228.157.53]:50620 "EHLO pb-smtp21.pobox.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S233104AbhETXzU (ORCPT ); Thu, 20 May 2021 19:55:20 -0400 Received: from pb-smtp21.pobox.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id B839B13B7A9; Thu, 20 May 2021 19:53:57 -0400 (EDT) (envelope-from junio@pobox.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=pobox.com; h=from:to:cc :subject:references:date:in-reply-to:message-id:mime-version :content-type:content-transfer-encoding; s=sasl; bh=CpinT+GIzpc5 3GDohXb3DolG7vwEYNLS1akhaCiua3M=; b=NOlFIO/Xp9qgIBtUSHz0OJL9+vYm FE7bWRh8xlJ3Dvxz70lZM+ps0ilTVy8NyMWidXdLQnDpYKmpPP9Huu4w0mNfuYPf 7WaVx6I9si5fe4Os+nb2X3e7LQUQyA4Iab2Ibbg2zyoUKp6xoPpjU6bGDb1bKkpK 17RN63fB9tpClYM= Received: from pb-smtp21.sea.icgroup.com (unknown [127.0.0.1]) by pb-smtp21.pobox.com (Postfix) with ESMTP id B122F13B7A8; Thu, 20 May 2021 19:53:57 -0400 (EDT) (envelope-from junio@pobox.com) Received: from pobox.com (unknown [34.73.10.127]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by pb-smtp21.pobox.com (Postfix) with ESMTPSA id 09CC713B7A7; Thu, 20 May 2021 19:53:54 -0400 (EDT) (envelope-from junio@pobox.com) From: Junio C Hamano To: Jeff King Cc: =?utf-8?B?w4Z2YXIgQXJuZmrDtnLDsA==?= Bjarmason , git@vger.kernel.org, Gregory Anders , =?utf-8?B?xJBvw6BuIFRy4bqnbiBDw7Ru?= =?utf-8?B?Zw==?= Danh , Eric Sunshine , Eric Wong Subject: Re: [PATCH v2 00/10] send-email: various optimizations to speed up by >2x References: Date: Fri, 21 May 2021 08:53:53 +0900 In-Reply-To: (Jeff King's message of "Thu, 20 May 2021 04:27:38 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 X-Pobox-Relay-ID: A2C75E88-B9C6-11EB-B9E0-D609E328BF65-77302942!pb-smtp21.pobox.com Content-Transfer-Encoding: quoted-printable Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Jeff King writes: > I like all of this, except for the change in the interface of > Git::config_regexp(). You mention that it's new-ish, and probably not i= n > wide use. And I agree that's probably true. But it feels like violating > a principle of not breaking APIs, and we should stick to that principle > and not bend it for "well, it's not that old an API". > > I'd find it more compelling if it the existing interface was broken or > hard to avoid changing. But couldn't we just add a new function with th= e > extra info (config_regexp_with_values() or something)? We seem to use it in-tree only once, but we have no control over what out-of-tree users have been doing. I do like your proposed name for the fact that it has _with_values in it; the original gave us only a list of configuration variable names, and now we are getting name-value tuples. BUT I am not sure if I like the new interface, and I do not know if the the name "config_with_values" is a good match for what it does. Namely, when a function is described as: =3Ditem config_regexp ( RE ) Retrieve the list of configuration key names matching the regular expression C. The return value is an ARRAY of key-value pairs. I would expect it to return ([key1, value1], [key2, value2], ...), but the implementation returns (key1, value1, key2, value2, ...), i.e. a flattened list, if I am not mistaken. my @cmd =3D ('config', '--null', '--get-regexp', $regex); unshift @cmd, $self if $self; my $data =3D command(@cmd); my (@kv) =3D map { split /\n/, $_, 2 } split /\0/, $data; return @kv; We get NUL terminated records, so we first split the entire output at NULs (to get a list of key-value records). Each key-value record has the key followed by LF followed by value, so we split it at the first LF (i.e. a value with an embedded LF would behave correctly) to extract key and value out of it. But the resulting list is a flat list with alternating key and value. The side that works on the returned value of courese knows that it is getting a flattened list and acts accordingly: my @kv =3D Git::config_regexp("^sende?mail[.]"); while (my ($k, $v) =3D splice @kv, 0, 2) { push @{$known_config_keys{$k}} =3D> $v; } Perhaps it may be more performant than a more obvious and straight-forward "list of tuples", i.e. return map { [split /\n/, $_, 2] } split /\0/, $data; my @kv =3D Git::config_regexp("^sende?mail[.]"); for my $tuple (@kv) { push @{$known_config_keys{$tuple->[0]}}, $tuple->[1]; } but somehow the flattened list felt unnatural at least to a na=C3=AFve reader like me.