mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Antonio Ospite <>
To: Junio C Hamano <>
Cc:, "Brandon Williams" <>,
	"Daniel Graña" <>,
	"Jonathan Nieder" <>,
	"Richard Hartmann" <>,
	"Stefan Beller" <>
Subject: Re: [RFC PATCH v2 06/12] submodule--helper: add a '--stage' option to the 'config' sub command
Date: Fri, 3 Aug 2018 13:03:34 +0200	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

On Thu, 02 Aug 2018 11:57:14 -0700
Junio C Hamano <> wrote:

> Antonio Ospite <> writes:
> > Add a --stage option to the 'submodule--helper config' command so that
> > the .gitmodules file can be staged without referring to it explicitly by
> > its file path.
> Sorry, but I have no clue what the above is trying to say.

You got me :), this was one of the changes I was having the more doubts
about, basically about the user interface, and it must have showed in
the commit message.

> The original 's--h config <name> [<value>]' is quite understandable.  It
> is like "git config <name> [<value>]", i.e. either get the current value
> for the name, or  set a new value to the name.
> What does this 's--h config --stage' that does not take any option
> do?  "git add .gitmodules"?  Something else?  In what meaning is the
> word "stage" used?  Is it used as a verb "to stage"?

"s--h config --stage" is meant to replace "git add -f .gitmodules" as
shown in patch 07, maybe the two patches might have been merged to give
more context, but I kept the changes separate to follow how things were
done with patches 04 and 05.

The rationale behind the change is to close the circle started with 04
and 05 and stop referring to .gitmodules explicitly by file path in git
commands. The change is not strictly needed for the series, it's for
consistency sake.

The name "stage" is meant as a verb and was inspired by the "git stage"
command and by the stage_updated_gitmodules() function, I used an
option flag with no arguments because I saw this has been done for
verbs before, e.g. "git config --list". I did not know about
"gitcli.txt", sorry, I'll check it out.

Maybe a better name could be "submodule--helper config --add"?

The commit message clearly needs improvements.

> In a series that lets us use the data in the .gitmodules file without
> materializing the file in the working tree, I would have expected
> that you would want an option to specify which .gitmodules among
> (1) the one in the working tree (i.e. the only option we currently
> have), (2) the one in the index, and (3) the one in the HEAD, and
> when I saw the title, I would have expected that
> 	git submodule--helper config --stage name
> may be a way to read from the .gitmodules in the index to find the
> value for name (however, we we follow the option naming convention
> in gitcli.txt, it should be called --cached, I would think).

For my use case the automatic behavior of falling back to
HEAD:.gitmodules is enough, so I focused on that to have the
functionality merged.

The option you suggest may be useful but I'd leave that as a possible
future addition in case someone else needed it.

> > In practice the config will still be written to .gitmodules, there are
> > no plans to change the file path, but having this level of indirection
> > makes it possible to perform additional checks before staging the file.
> Again, a claim without explanation or justification.
> If you are planning to something like
>  - prepare trial contents in file
>  - do whatever "additional checks" on
>  - add the contents to it to the index as a new .gitmodules blob
> Then you do not need such an option.  "submodule--helper" is purely
> a helper for scripts, and not for human consumption, so scripts can
> just hash-object the blob contents out and update-index --cacheinfo
> to register the blob at any location of choice.
> But as I said, this step is way under-explained, so my guess above
> may not match what you really wanted to do.

As stated, the motivation for now is just syntactical: remove hardcoded
references to .gitmodules in scripts.

No new "additional checks" are added as of now, the commit message is
misleading indeed.


Antonio Ospite

A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

  reply	other threads:[~2018-08-03 11:03 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-08-02 13:46 [RFC PATCH v2 00/12] Make submodules work if .gitmodules is not checked out Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 01/12] submodule: add a print_config_from_gitmodules() helper Antonio Ospite
2018-08-02 18:05   ` Stefan Beller
2018-08-09 10:17     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 02/12] submodule: factor out a config_set_in_gitmodules_file_gently function Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 03/12] t7411: be nicer to future tests and really clean things up Antonio Ospite
2018-08-02 16:40   ` SZEDER Gábor
2018-08-02 18:15     ` Stefan Beller
2018-08-09 13:59       ` Antonio Ospite
2018-08-02 18:44     ` Junio C Hamano
2018-08-02 13:46 ` [RFC PATCH v2 04/12] submodule--helper: add a new 'config' subcommand Antonio Ospite
2018-08-02 18:47   ` Stefan Beller
2018-08-02 19:20     ` Jeff King
2018-08-03 10:21       ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 05/12] submodule: use the 'submodule--helper config' command Antonio Ospite
2018-08-02 18:59   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 06/12] submodule--helper: add a '--stage' option to the 'config' sub command Antonio Ospite
2018-08-02 18:57   ` Junio C Hamano
2018-08-03 11:03     ` Antonio Ospite [this message]
2018-08-03 16:24       ` Junio C Hamano
2018-08-06 10:58         ` Antonio Ospite
2018-08-06 17:38           ` Junio C Hamano
2018-08-07  9:19             ` Antonio Ospite
2018-08-06 18:19           ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 07/12] submodule: use 'submodule--helper config --stage' to stage .gitmodules Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 08/12] t7506: cleanup .gitmodules properly before setting up new scenario Antonio Ospite
2018-08-02 19:11   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 09/12] submodule: support reading .gitmodules even when it's not checked out Antonio Ospite
2018-08-02 20:27   ` Stefan Beller
2018-08-02 13:46 ` [RFC PATCH v2 10/12] t7416: add new test about HEAD:.gitmodules and not existing .gitmodules Antonio Ospite
2018-08-02 20:43   ` Stefan Beller
2018-08-09  9:14     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 11/12] dir: move is_empty_file() from builtin/am.c to dir.c and make it public Antonio Ospite
2018-08-02 20:50   ` Stefan Beller
2018-08-03  8:49     ` Antonio Ospite
2018-08-02 13:46 ` [RFC PATCH v2 12/12] submodule: remove the .gitmodules file when it is empty Antonio Ospite
2018-08-02 21:15   ` Stefan Beller
2018-08-03  8:57     ` Antonio Ospite

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:

  List information:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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).