git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Antonio Ospite <ao2@ao2.it>
To: Stefan Beller <sbeller@google.com>
Cc: git <git@vger.kernel.org>, Brandon Williams <bmwill@google.com>,
	Daniel Graña <dangra@gmail.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Richard Hartmann <richih.mailinglist@gmail.com>
Subject: Re: [RFC PATCH 04/10] submodule--helper: add a new 'config' subcommand
Date: Wed, 20 Jun 2018 23:32:39 +0200
Message-ID: <20180620233239.dce00f3d9400b674f95adad7@ao2.it> (raw)
In-Reply-To: <CAGZ79kbkDO8KEm6ps75xTykU2JCVwHVVpOr+Eq_RQy_bn-jRCw@mail.gmail.com>

On Mon, 14 May 2018 18:33:44 -0700
Stefan Beller <sbeller@google.com> wrote:

> On Mon, May 14, 2018 at 3:58 AM, Antonio Ospite <ao2@ao2.it> wrote:
> > Add a new 'config' subcommand to 'submodule--helper', this extra level
> > of indirection makes it possible to add some flexibility to how the
> > submodules configuration is handled.
> >
> > Signed-off-by: Antonio Ospite <ao2@ao2.it>
> > ---
> >  builtin/submodule--helper.c | 39 +++++++++++++++++++++++++++++++++++++
> >  t/t7411-submodule-config.sh | 26 +++++++++++++++++++++++++
> >  2 files changed, 65 insertions(+)
> >
> > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> > index 9e8f2acd5..b32110e3b 100644
> > --- a/builtin/submodule--helper.c
> > +++ b/builtin/submodule--helper.c
> > @@ -1825,6 +1825,44 @@ static int is_active(int argc, const char **argv, const char *prefix)
> >         return !is_submodule_active(the_repository, argv[1]);
> >  }
> >
> > +static int config_print_callback(const char *key_, const char *value_, void *cb_data)
> > +{
> > +       char *key = cb_data;
> > +
> > +       if (!strcmp(key, key_))
> > +               printf("%s\n", value_);
> > +
> > +       return 0;
> > +}
> > +
> > +static int module_config(int argc, const char **argv, const char *prefix)
> > +{
> > +       int ret;
> > +
> > +       if (argc < 2 || argc > 3)
> > +               die("submodule--helper config takes 1 or 2 arguments: name [value]");
> > +
> > +       /* Equivalent to ACTION_GET in builtin/config.c */
> > +       if (argc == 2) {
> > +               char *key;
> > +
> > +               ret = git_config_parse_key(argv[1], &key, NULL);
> > +               if (ret < 0)
> > +                       return CONFIG_INVALID_KEY;
> > +
> > +               config_from_gitmodules(config_print_callback, the_repository, key);
> > +
> > +               free(key);
> > +               return 0;
> > +       }
> > +
> > +       /* Equivalent to ACTION_SET in builtin/config.c */
> > +       if (argc == 3)
> > +               return config_gitmodules_set(argv[1], argv[2]);
> 
> Ah, here we definitely want to set it in the .gitmodules file?
> (Or does that change later in this series?)
> 
> > +
> > +       return 0;
> > +}
> > +
> >  #define SUPPORT_SUPER_PREFIX (1<<0)
> >
> >  struct cmd_struct {
> > @@ -1850,6 +1888,7 @@ static struct cmd_struct commands[] = {
> >         {"push-check", push_check, 0},
> >         {"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
> >         {"is-active", is_active, 0},
> > +       {"config", module_config, 0},
> >  };
> >
> >  int cmd_submodule__helper(int argc, const char **argv, const char *prefix)
> > diff --git a/t/t7411-submodule-config.sh b/t/t7411-submodule-config.sh
> > index a648de6a9..dfe019f05 100755
> > --- a/t/t7411-submodule-config.sh
> > +++ b/t/t7411-submodule-config.sh
> > @@ -139,4 +139,30 @@ test_expect_success 'error in history in fetchrecursesubmodule lets continue' '
> >         )
> >  '
> >
> > +test_expect_success 'reading submodules config with "submodule--helper config"' '
> > +       (cd super &&
> 
> I think the project prefers a style
> of the cd at the same level of the echo and the following commands.
>

There is mixed style about that, so for new tests in existing files I'd
stick to the predominant style in the file.

For new test files I'll use the recommended style of cd on the same
level of the following commands.

> However we might not need the (cd super && ...) via
> 
>   echo "../submodule"  >expected
>   git -C super ubmodule--helper config submodule.submodule.url >../actual
>   test_cmp expected actual
> 
> Our friends developing git on Windows will thank us for saving
> to spawn a shell as spawning processes is expensive on Windows. :)
> Also we have fewer lines of code.
>

I'll see how that looks, thanks for the suggestion.

Again, I'd use a subshell if that's what the other tests in a file do,
and use "git -C" in new files.

Thanks,
   Antonio

-- 
Antonio Ospite
https://ao2.it
https://twitter.com/ao2it

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?

  reply index

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-14 10:58 [RFC PATCH 00/10] Make submodules work if .gitmodules is not checked out Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 01/10] config: make config_from_gitmodules generally useful Antonio Ospite
2018-05-14 18:19   ` Brandon Williams
2018-06-20 18:04     ` Antonio Ospite
2018-05-15  1:05   ` Stefan Beller
2018-06-20 18:06     ` Antonio Ospite
2018-06-20 19:10       ` Stefan Beller
2018-06-21 13:54         ` Antonio Ospite
2018-06-21 18:53           ` Stefan Beller
2018-05-14 10:58 ` [RFC PATCH 02/10] submodule: factor out a config_gitmodules_set function Antonio Ospite
2018-05-15  1:20   ` Stefan Beller
2018-06-20 18:41     ` Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 03/10] t7411: be nicer to other tests and really clean things up Antonio Ospite
2018-05-15  1:23   ` Stefan Beller
2018-06-20 21:16     ` Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 04/10] submodule--helper: add a new 'config' subcommand Antonio Ospite
2018-05-15  1:33   ` Stefan Beller
2018-06-20 21:32     ` Antonio Ospite [this message]
2018-05-14 10:58 ` [RFC PATCH 05/10] submodule: use the 'submodule--helper config' command Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 06/10] submodule--helper: add a '--stage' option to the 'config' sub command Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 07/10] submodule: use 'submodule--helper config --stage' to stage .gitmodules Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 08/10] t7506: cleanup .gitmodules properly before setting up new scenario Antonio Ospite
2018-05-14 10:58 ` [RFC PATCH 09/10] submodule: support reading .gitmodules even when it's not checked out Antonio Ospite
2018-05-15  1:45   ` Stefan Beller
2018-05-14 10:58 ` [RFC PATCH 10/10] t7415: add new test about using HEAD:.gitmodules from the index Antonio Ospite
2018-05-15  1:14 ` [RFC PATCH 00/10] Make submodules work if .gitmodules is not checked out Stefan Beller
2018-05-15  4:09 ` Junio C Hamano

Reply instructions:

You may reply publically 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=20180620233239.dce00f3d9400b674f95adad7@ao2.it \
    --to=ao2@ao2.it \
    --cc=bmwill@google.com \
    --cc=dangra@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jrnieder@gmail.com \
    --cc=richih.mailinglist@gmail.com \
    --cc=sbeller@google.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 mailing list mirror (one of many)

Archives are clonable:
	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

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.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox