Re: [GSOC PATCH 2/2] builtin/fmt-merge-msg: stop depending on 'the_repository'
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 29, 2025, 16:41 UTC
- Message-ID
- <xmqqjz3rospl.fsf@gitster.g>
- In-Reply-To
- <04d6f682a6b2257e14682e809a2fd01ccfcf0d08.1753804956.git.ayu.chandekar@gmail.com>
Ayush Chandekar <ayu.chandekar@gmail.com> writes:
Show 6 quoted lines
> Refactor builtin/fmt-merge-msg.c to remove the dependancy on the global > 'the_repository'. Replace all the occurrences of 'the_repository' with > 'repo', where 'repo' is a pointer to 'struct repository' passed to the > function 'cmd_fmt_merge_msg()' and thus remove the definition '#define > USE_THE_REPOSITORY_VARIABLE'. Also, add a test to make sure that "git > fmt-merge-msg -h" can be called outside a repository.
This also moves the call to git_config()/repo_config() after parse_options().
It generally is a bad idea to read command line options first and then read the configuration (it is a bug if such a flow causes values from configuration to overwrite values from command line). THe current set of options and configuration variables may not overlap, in which case such a questionable arrangement happen to be without bug right now, but it would prevent future developers from adding new options and configuration variables and make them interact with each other in the most natural way.
In any case, the reason for this change of the order between config and parse-options is not explained at all in the proposed log message.
Show 58 quoted lines
> Mentored-by: Christian Couder <christian.couder@gmail.com>
> Mentored-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
> Signed-off-by: Ayush Chandekar <ayu.chandekar@gmail.com>
> ---
> builtin/fmt-merge-msg.c | 7 +++----
> t/t1517-outside-repo.sh | 7 +++++++
> 2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
> index fed8163825..848498b8e6 100644
> --- a/builtin/fmt-merge-msg.c
> +++ b/builtin/fmt-merge-msg.c
> @@ -1,4 +1,3 @@
> -#define USE_THE_REPOSITORY_VARIABLE
> #include "builtin.h"
> #include "config.h"
> #include "fmt-merge-msg.h"
> @@ -13,7 +12,7 @@ static const char * const fmt_merge_msg_usage[] = {
> int cmd_fmt_merge_msg(int argc,
> const char **argv,
> const char *prefix,
> - struct repository *repo UNUSED)
> + struct repository *repo)
> {
> char *inpath = NULL;
> const char *message = NULL;
> @@ -53,13 +52,13 @@ int cmd_fmt_merge_msg(int argc,
> int ret;
> struct fmt_merge_msg_opts opts;
>
> - git_config(fmt_merge_msg_config, NULL);
> argc = parse_options(argc, argv, prefix, options, fmt_merge_msg_usage,
> 0);
> if (argc > 0)
> usage_with_options(fmt_merge_msg_usage, options);
> + repo_config(repo, fmt_merge_msg_config, NULL);
>
> - adjust_shortlog_len(the_repository, &shortlog_len);
> + adjust_shortlog_len(repo, &shortlog_len);
>
> if (inpath && strcmp(inpath, "-")) {
> in = fopen(inpath, "r");
> diff --git a/t/t1517-outside-repo.sh b/t/t1517-outside-repo.sh
> index 8f59b867f2..4b4e645860 100755
> --- a/t/t1517-outside-repo.sh
> +++ b/t/t1517-outside-repo.sh
> @@ -121,4 +121,11 @@ test_expect_success 'prune does not crash with -h' '
> test_grep "[Uu]sage: git prune " usage
> '
>
> +test_expect_success 'fmt-merge-msg does not crash with -h' '
> + test_expect_code 129 git fmt-merge-msg -h >usage &&
> + test_grep "[Uu]sage: git fmt-merge-msg " usage &&
> + test_expect_code 129 nongit git fmt-merge-msg -h >usage &&
> + test_grep "[Uu]sage: git fmt-merge-msg " usage
> +'
> +
> test_done