From: Emily Shaffer <emilyshaffer@google.com> To: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com> Cc: git@vger.kernel.org, "Jonathan Nieder" <jrnieder@gmail.com>, "Johannes Schindelin" <Johannes.Schindelin@gmx.de>, "Jeff King" <peff@peff.net>, "brian m. carlson" <sandals@crustytoothpaste.net>, "Martin Ågren" <martin.agren@gmail.com>, "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>, "Derrick Stolee" <stolee@gmail.com>, "Derrick Stolee" <derrickstolee@github.com>, "Derrick Stolee" <dstolee@microsoft.com> Subject: Re: [PATCH v2 1/7] t1300: test "set all" mode with value_regex Date: Mon, 23 Nov 2020 11:37:14 -0800 Message-ID: <20201123193714.GA499823@google.com> (raw) In-Reply-To: <ea3099719c867a5a475468ddb6741bbcd713dccc.1606147507.git.gitgitgadget@gmail.com> On Mon, Nov 23, 2020 at 04:05:01PM +0000, Derrick Stolee via GitGitGadget wrote: > Add a test that demonstrates these options. Break from the existing > pattern in t1300-config.sh to use 'git config --file=<file>' instead of > modifying .git/config directly. Also use 'git config --file=<file> > --list' for config state comparison instead of the config file format. I'd normally say "but *why* do you want to do that?" but I think it's actually pretty clear in the diff based on the naming of the config files (and clever - I like it). I do think pointing out that you're breaking from existing style is useful since the existing style isn't contained in the diff alone. > +test_expect_success 'set all config with value_regex' ' > + git config --file=initial abc.key one && > + > + cp initial config && > + git config --file=config abc.key two a+ && Make sure that if the value_regex misses existing keys, it can still add normally. > + git config --file=config --list >actual && > + cat >expect <<-\EOF && > + abc.key=one > + abc.key=two > + EOF > + test_cmp expect actual && > + > + test_must_fail git config --file=config abc.key three o+ 2>err && > + test_i18ngrep "has multiple values" err && Try to change the value of the config, but it's a multiconfig already. Ok. > + git config --file=config abc.key three a+ && But /a+/ doesn't match any configs that already exist, so this one gets added fine. > + git config --file=config --list >actual && > + cat >expect <<-\EOF && > + abc.key=one > + abc.key=two > + abc.key=three > + EOF > + test_cmp expect actual && > + > + cp initial config && Get back the config before the last exercise (with just 'one'). > + git config --file=config abc.key three o+ && And if the value_regex matches exactly one config, then replace that one. I could see this case being a little bit more compelling if the value_regex was doing some lifting, e.g. abc.key=nil abc.key=one git config abc.key two o+ abc.key=nil abc.key=two This criteria was kind of implied in #2 in your commit message - does it work? > + git config --file=config --list >actual && > + cat >expect <<-\EOF && > + abc.key=three > + EOF > + test_cmp expect actual > +' Anyway, it's a pain, but I'd like to see either comments describing which case each one is, or individual tests. - Emily
next prev parent reply other threads:[~2020-11-23 19:42 UTC|newest] Thread overview: 72+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-19 15:52 [PATCH 0/7] config: add --literal-value option Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 1/7] t1300: test "set all" mode with value_regex Derrick Stolee via GitGitGadget 2020-11-19 22:24 ` Junio C Hamano 2020-11-20 2:09 ` brian m. carlson 2020-11-20 2:33 ` Junio C Hamano 2020-11-20 18:39 ` Jeff King 2020-11-20 22:35 ` Junio C Hamano 2020-11-21 22:27 ` brian m. carlson 2020-11-22 3:31 ` Junio C Hamano 2020-11-24 2:38 ` Jeff King 2020-11-24 19:43 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 2/7] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 3/7] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-19 22:32 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 4/7] config: add --literal-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-19 22:42 ` Junio C Hamano 2020-11-20 6:35 ` Martin Ågren 2020-11-19 15:52 ` [PATCH 5/7] config: plumb --literal-value into config API Derrick Stolee via GitGitGadget 2020-11-19 22:45 ` Junio C Hamano 2020-11-19 15:52 ` [PATCH 6/7] config: implement --literal-value with --get* Derrick Stolee via GitGitGadget 2020-11-19 15:52 ` [PATCH 7/7] maintenance: use 'git config --literal-value' Derrick Stolee via GitGitGadget 2020-11-19 23:17 ` Junio C Hamano 2020-11-20 13:19 ` [PATCH 0/7] config: add --literal-value option Ævar Arnfjörð Bjarmason 2020-11-20 13:23 ` Derrick Stolee 2020-11-20 18:30 ` Junio C Hamano 2020-11-20 18:51 ` Derrick Stolee 2020-11-20 21:52 ` Junio C Hamano 2020-11-24 12:35 ` Ævar Arnfjörð Bjarmason 2020-11-23 16:05 ` [PATCH v2 0/7] config: add --fixed-value option Derrick Stolee via GitGitGadget 2020-11-23 16:05 ` [PATCH v2 1/7] t1300: test "set all" mode with value_regex Derrick Stolee via GitGitGadget 2020-11-23 19:37 ` Emily Shaffer [this message] 2020-11-23 16:05 ` [PATCH v2 2/7] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-23 19:40 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 3/7] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-23 21:43 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 4/7] config: add --fixed-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-23 19:37 ` Junio C Hamano 2020-11-23 21:51 ` Emily Shaffer 2020-11-23 22:41 ` Junio C Hamano 2020-11-25 14:08 ` Derrick Stolee 2020-11-25 17:22 ` Derrick Stolee 2020-11-25 17:28 ` Eric Sunshine 2020-11-25 19:30 ` Junio C Hamano 2020-11-25 19:29 ` Junio C Hamano 2020-11-23 16:05 ` [PATCH v2 5/7] config: plumb --fixed-value into config API Derrick Stolee via GitGitGadget 2020-11-23 22:21 ` Emily Shaffer 2020-11-24 0:52 ` Eric Sunshine 2020-11-25 15:41 ` Derrick Stolee 2020-11-25 17:55 ` Eric Sunshine 2020-11-23 16:05 ` [PATCH v2 6/7] config: implement --fixed-value with --get* Derrick Stolee via GitGitGadget 2020-11-23 19:53 ` Junio C Hamano 2020-11-23 22:43 ` Emily Shaffer 2020-11-23 16:05 ` [PATCH v2 7/7] maintenance: use 'git config --fixed-value' Derrick Stolee via GitGitGadget 2020-11-23 21:39 ` Junio C Hamano 2020-11-23 22:48 ` Emily Shaffer 2020-11-23 23:27 ` Junio C Hamano 2020-11-23 19:33 ` [PATCH v2 0/7] config: add --fixed-value option Junio C Hamano 2020-11-25 22:12 ` [PATCH v3 0/8] " Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 1/8] config: convert multi_replace to flags Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 2/8] config: replace 'value_regex' with 'value_pattern' Derrick Stolee via GitGitGadget 2020-11-25 22:50 ` Eric Sunshine 2020-11-25 22:12 ` [PATCH v3 3/8] t1300: test "set all" mode with value-pattern Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 4/8] t1300: add test for --replace-all " Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 5/8] config: add --fixed-value option, un-implemented Derrick Stolee via GitGitGadget 2020-11-25 23:04 ` Eric Sunshine 2020-11-25 22:12 ` [PATCH v3 6/8] config: plumb --fixed-value into config API Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 7/8] config: implement --fixed-value with --get* Derrick Stolee via GitGitGadget 2020-11-25 22:12 ` [PATCH v3 8/8] maintenance: use 'git config --fixed-value' Derrick Stolee via GitGitGadget 2020-11-25 23:09 ` Junio C Hamano 2020-11-25 23:00 ` [PATCH v3 0/8] config: add --fixed-value option Junio C Hamano 2020-11-26 11:17 ` Derrick Stolee 2020-12-01 4:45 ` 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=20201123193714.GA499823@google.com \ --to=emilyshaffer@google.com \ --cc=Johannes.Schindelin@gmx.de \ --cc=avarab@gmail.com \ --cc=derrickstolee@github.com \ --cc=dstolee@microsoft.com \ --cc=git@vger.kernel.org \ --cc=gitgitgadget@gmail.com \ --cc=jrnieder@gmail.com \ --cc=martin.agren@gmail.com \ --cc=peff@peff.net \ --cc=sandals@crustytoothpaste.net \ --cc=stolee@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
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