| Message ID | pull.1681.git.1709532018372.gitgitgadget@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Allow git-config to append a comment | expand |
"Ralph Seichter via GitGitGadget" <gitgitgadget@gmail.com> writes: > From: Ralph Seichter <github@seichter.de> > Subject: Re: [PATCH] Allow git-config to append a comment Thanks for a patch. Make sure your title will mix well in "git shortlog --no-merges" output from recent commits by other contributors. See Documentation/SubmittingPatches[[describe-changes]]. Perhaps Subject: [PATCH] config: add --comment option to add a comment > Introduce the ability to append comments to modifications > made using git-config. Example usage: > > git config --comment "I changed this. --A. Script" \ > --add safe.directory /home/alice/somerepo.git If you are illustrating a sample input, please also explain what output it produces. What do the resulting lines in the config file look like after you run this command? I find the overall idea somewhat incomplete. Perhaps stepping back a bit and thinking through the _whole_ use case of "comment" in the configuration files is in order, before thinking about an easier way to add one. Why are you adding "# comment" to your config file? Who reads these comments, with what tool, and for what purpose? What will they do after they learn something from what their former selves wrote as comments? Let's imagine we had this in our global configuration file in the beginning: $ cat ~/.gitconfig ... [safe] directory = /home/bob/otherrepo.git ... and then we run this: $ git config --global --comment 'the reason why I added ~alice/ is because ...' --add safe.directory /home/alice/somerepo.git $ git config --global --add safe.directory /home/charlie/somerepo.git which may modify the configuration file with the new entry with a comment. The part about safe.directory might now read like so: [safe] directory = /home/bob/otherrepo.git # the reason why I added ~alice/ # is because ... directory = /home/alice/somerepo.git directory = /home/charlie/somerepo.git Now how do we find out about this comment? "git config -l" would not give us that. Are we expected to look at it in our editor or pager? $ vi ~/.gitconfig $ less ~/.gitconfig Why are we interested in looking at these comments in the first place [*]? Side note: I do not ask "why are we interested in leaving these comments in the file"---it goes without saying that it is because we want to be able to later read them. Presumably, we are trying to remind ourselves why we added a particular variable=value. By learning that, what are we going to do next? Perhaps once the reason to mark /home/alice/somerepo.git as a safe.directory disappears, we would want to remove this entry? But then, after running $ less ~/.gitconfig and then learning that we no longer need /home/alice/somerepo.git marked as a safe.directory, how would we remove it from the configuration file, and what happens to the comment? $ git config --global --unset safe.directory /home/alice/somerepo.git may remove the value, but it would not remove the comment, and you'll be left with something like this: [safe] directory = /home/bob/otherrepo.git # the reason why I added ~alice/ # is because ... directory = /home/charlie/somerepo.git The remaining comment looks as if it talks about "charlie", but it simply is stale. Should it have removed the comment, then? I actually do not think so. A comment before a particular variable definition may not be the result of the use of this new "config --comment" option, but perhaps the user added it manually. Or even worse, the comment may not even be about an individual entry. Imagine [section] # definitions for per-user stuff begin here var = /home/alice # these two directories are actually charlie's var = /home/bob var = /home/charlie # definitions for per-project stuff var = /project/a var = /project/b Can we come up with a code that reliably decides when to remove the first comment we see above? The answer given by human might be "when all the entries about /home/alice, /home/bob, and /home/charlie get removed", but can we implement that in code without interpreting the natural language? I doubt it. We may even be better off to keep the comment to remind ourselves where to add the per-user stuff in the section when we hire a new user the next time. With the new "--comment" thing might be able to add comments without using an editor, but the user would need to view the configuration file with a pager or an editor bypassing Git to make use of what was recorded in there. And it is likely that these comments will need to be edited or removed using an editor because "git config" command would not be usable for maintaining them. For that matter, if you made a typo in your "--comment" option from the command line, you would have to resort to your editor to fix that typo. The above is an illustration of what I want to see the author, who comes up with this "wouldn't it be wonderful to teach git-config to add comment?" idea, thinks through. The first patch might be to just add a comment when a variable=value is added, but we want to see the vision of how the whole end-user experience should look like, in which this first small piece will fit. Identify the whole life cycle of the result of using the feature, what the end-user experience of using the result of using the feature would look like, etc. In this case, "the result of using the feature" is the comments that are initially placed next to a newly defined variable-value pair, and at least to me, the end-user experience to manage the life cycle of these comments is not all that improved from the status quo, which is that you need your pager and editor to view them, edit them, typofix them, and remove them. I am somewhat negative on the idea to make the initial addition of the comment vastly different from all other uses of the comment by introducing this "--comment" option. If we have a vision, if not design and/or code, to help managing other parts of the lifecycle, not just adding in the beginning, it might be a different story, but I cannot quite see it. > Documentation/git-config.txt | 10 +++++++--- > builtin/config.c | 21 ++++++++++++++------- > builtin/gc.c | 4 ++-- > builtin/submodule--helper.c | 2 +- > builtin/worktree.c | 4 ++-- > config.c | 21 +++++++++++++-------- > config.h | 4 ++-- > sequencer.c | 28 ++++++++++++++-------------- > submodule-config.c | 2 +- > submodule.c | 2 +- > t/t1300-config.sh | 9 +++++++-- > worktree.c | 4 ++-- > 12 files changed, 66 insertions(+), 45 deletions(-) And the amount of the change required for that tiny bit of "improvement" (if we can call it, which is dubious) does not seem worth it. Thanks.
* Junio C Hamano: > Make sure your title will mix well in "git shortlog --no-merges" > output from recent commits by other contributors. Thank you for your in-depth comment. This is the first time I have ever considered contributing to Git, so I have a lot to learn. My pull request [1] on GitGitGadget has been approved by Johannes Schindelin, by the way, and the PR is based on an issue Johannes created [2] after a brief discussion him and I had on Discord [3]. I have updated the subject line, as you suggested. [1] https://github.com/gitgitgadget/git/pull/1681 [2] https://github.com/gitgitgadget/git/issues/1680 [3] https://discord.com/channels/1042895022950994071/1213052410281467906 I don't know if it is the polite thing to ask you to please refer to the information I linked, or if I should duplicate the information here? The short version is that I need to be able to distinguish between config entries made by automation (like scripts, or Ansible in my particular case) and those made by a human. >> git config --comment "I changed this. --A. Script" \ >> --add safe.directory /home/alice/somerepo.git > > If you are illustrating a sample input, please also explain what > output it produces. What do the resulting lines in the config file > look like after you run this command? The result of running the above command looks as follows: [safe] directory = /home/alice/somerepo.git #I changed this. --A. Script > Why are you adding "# comment" to your config file? Who reads these > comments, with what tool, and for what purpose? I mentioned human-readable comments in the patch, and humans are indeed the intended audience. If a human finds changes made to a Git config file, and a comment states that the modification was e.g. made by automation, it adds beneficial information for said human. I can for example create a comment with a URL pointing to a website providing additional explanations. > Now how do we find out about this comment? "git config -l" would > not give us that. Are we expected to look at it in our editor or > pager? Yes. I routinely use cat/vim to inspect/modify my Git config files. They are suitable for human consumption, after all. Also, comments can already be manually added in creative ways, and are ignored when Git reads config data. Comments being read only by humans is pretty much their whole point, in my opinion. > Can we come up with a code that reliably decides when to remove the > first comment we see above? Your examples about difficulties removing comments hinge on there being multiline comments, as far as I can tell. My patch only supports single-line comments, and only as a suffix to newly added key-value pairs. This is a deliberate design choice. > The above is an illustration of what I want to see the author, who > comes up with this "wouldn't it be wonderful to teach git-config to > add comment?" idea, thinks through. The first patch might be to > just add a comment when a variable=value is added, but we want to > see the vision of how the whole end-user experience should look > like, in which this first small piece will fit. I don't have a greater vision for comments. Their use is to provide information for humans, no more, no less. There is also no idea of a user experience beyond pager/editor in my mind. The patch addresses specific needs of mine, and Johannes suggested it as a new feature, and that's all the motivation behind it. > And the amount of the change required for that tiny bit of > "improvement" (if we can call it, which is dubious) does not seem > worth it. As I mentioned in my PR, it also does not seem elegant to me to modify so many files. Alas, C does not offer expanding function signatures by adding parameters with default values, like Python does. Adding a new function like, perhaps, git_config_set_in_file_gently_with_comment() could be a remedy? -Ralph
Ralph Seichter <github@seichter.de> writes: >> If you are illustrating a sample input, please also explain what >> output it produces. What do the resulting lines in the config file >> look like after you run this command? > > The result of running the above command looks as follows: > > [safe] > directory = /home/alice/somerepo.git #I changed this. --A. Script That would have been a crucial piece of information to have in the proposed log message, as limiting ourselves to a comment that is tucked after the same line as the value, things can become somewhat simplified. We may not have to worry about deletion, even though the point about "we need to look at and typofix them with our viewers and editors" still stands. By the way, you may or may not have noticed it, but my example deliberately had a multi-line comment: $ git config --global --comment 'the reason why I added ~alice/ is because ...' --add safe.directory /home/alice/somerepo.git How such a thing is handled also needs to be discussed in the proposed log message, and perhaps in the documentation as well. > ... My patch only supports > single-line comments, and only as a suffix to newly added key-value > pairs. This is a deliberate design choice. Such design choices need to be described in the proposed log message to help future developers who will be updating this feature, once it gets in. Thanks for writing quite a lot to answer _my_ questions, but these questions are samples of things that future developers would wonder and ask about when they want to fix bugs in, enhance, or otherwise modify the implementation of this "add comment" feature. They may even be working on adding other features to complement the "add comment" feature, by designing support for viewing or typofixing existing comments. When they do so, it would help them to know how this existing feature was expected to be used and how it would fit in a larger picture (which may not have yet existed back when the feature was invented). Answering these anticipated questions is one of the greatest things that a commit log message can do to help them. Thanks.
* Junio C. Hamano: >> My patch only supports single-line comments, and only as a suffix to >> newly added key-value pairs. This is a deliberate design choice. > > Such design choices need to be described in the proposed log message > to help future developers who will be updating this feature, once it > gets in. Just as a brief interjection: I am sorry that my inexperience with Git's mailing-list based process caused me to leave out details which were discussed earlier, be it on GitHub or Discord. However, me not mentioning that I am aiming for single-line suffix comments only was due to simple forgetfulness, without an excuse behind it. My bad. I think it best I flesh out the commit message before anything else, and then resubmit. That should bundle the necessary information. -Ralph
Ralph Seichter <ralph@seichter.de> writes: > Just as a brief interjection: I am sorry that my inexperience with Git's > mailing-list based process caused me to leave out details which were > discussed earlier, be it on GitHub or Discord. However, me not > mentioning that I am aiming for single-line suffix comments only was due > to simple forgetfulness, without an excuse behind it. My bad. Don't feel too bad. It is not about discord vs mailing list vs github "reviews on the web". It is about the end-product, the commit log messages, what future developers will see in their "git log" output. > I think it best I flesh out the commit message before anything else, and > then resubmit. That should bundle the necessary information. Yup. Please do that. Communicating with reviewers and other contributors is not the primary goal of the review process. It is done to help us reach a better end-product, the commit log messages that will help our future developers and future selves. Thanks.
On Thursday, March 7, 2024 7:12 AM, Junio C Hamano wrote: >Ralph Seichter <github@seichter.de> writes: > >>> If you are illustrating a sample input, please also explain what >>> output it produces. What do the resulting lines in the config file >>> look like after you run this command? >> >> The result of running the above command looks as follows: >> >> [safe] >> directory = /home/alice/somerepo.git #I changed this. --A. Script > >That would have been a crucial piece of information to have in the proposed log message, as limiting ourselves to a comment that is >tucked after the same line as the value, things can become somewhat simplified. We may not have to worry about deletion, even >though the point about "we need to look at and typofix them with our viewers and editors" still stands. > >By the way, you may or may not have noticed it, but my example deliberately had a multi-line comment: > > $ git config --global --comment 'the reason why I added ~alice/ > is because ...' --add safe.directory /home/alice/somerepo.git > >How such a thing is handled also needs to be discussed in the proposed log message, and perhaps in the documentation as well. > >> ... My patch only supports >> single-line comments, and only as a suffix to newly added key-value >> pairs. This is a deliberate design choice. > >Such design choices need to be described in the proposed log message to help future developers who will be updating this feature, >once it gets in. > >Thanks for writing quite a lot to answer _my_ questions, but these questions are samples of things that future developers would >wonder and ask about when they want to fix bugs in, enhance, or otherwise modify the implementation of this "add comment" >feature. They may even be working on adding other features to complement the "add comment" feature, by designing support for >viewing or typofixing existing comments. When they do so, it would help them to know how this existing feature was expected to be >used and how it would fit in a larger picture (which may not have yet existed back when the feature was invented). Answering these >anticipated questions is one of the greatest things that a commit log message can do to help them. > >Thanks. I am concerned about the compatibility of this series with the community. While comments are permitted in .gitconfig files, I am not 100% sure that all stakeholders, particularly those who parse .gitconfig files in their own scripts outside of git - sure, it is their own responsibility, but this might be unexpected. I worry that this might unintentionally introduce incompatibilities into repository configurations. Is there broad consensus to do this? --Randall
* rsbecker@nexbridge.com: > While comments are permitted in .gitconfig files, I am not 100% sure > that all stakeholders, particularly those who parse .gitconfig files > in their own scripts outside of git - sure, it is their own > responsibility, but this might be unexpected. Comments are nothing new, and humans have added far crazier comments to their Git config in the past. The patch ensures that a '#' precedes the comments added using git-config, which is not guaranteed to happen when Joe Random User manually edits config files. I think that anybody incapable of reliably dealing with comments in config files would already have fallen flat on his/her nose, regardless of how those comments were made. > I worry that this might unintentionally introduce incompatibilities > into repository configurations. Do you have an example? -Ralph
On Thursday, March 7, 2024 10:26 AM, Ralph Seichter wrote: >* rsbecker@nexbridge.com: > >> While comments are permitted in .gitconfig files, I am not 100% sure >> that all stakeholders, particularly those who parse .gitconfig files >> in their own scripts outside of git - sure, it is their own >> responsibility, but this might be unexpected. > >Comments are nothing new, and humans have added far crazier comments to their Git config in the past. The patch ensures that a '#' >precedes the comments added using git-config, which is not guaranteed to happen when Joe Random User manually edits config files. > >I think that anybody incapable of reliably dealing with comments in config files would already have fallen flat on his/her nose, >regardless of how those comments were made. > >> I worry that this might unintentionally introduce incompatibilities >> into repository configurations. > >Do you have an example? No example. This is a comment on "potential" changes to data that scripts around git for automation purposes might use. My purpose is just to highlight, for the purpose of reviewing the change, that there may be unintended impacts, that's all. It may be useful to include comments in the change notices and documentation pages that using this capability may impact scripting. When a user manually puts in a comment, any breakages in their scripts are 100% their issue. With git config moving comments around, responsibility shifts to git - a.k.a., unintended consequences. I am not asking that this change not happen - it is a good thing, but ensuring that we communicate that this may cause breakages if external programs/scripts read .gitconfig would be helpful. This also would need to be coordinated with the libification efforts at some point.
* rsbecker@nexbridge.com: > My purpose is just to highlight, for the purpose of reviewing the > change, that there may be unintended impacts, that's all. I see. Have you perhaps spotted a flaw in the patch which might cause problems? If so, I'd like to address it. > With git config moving comments around, responsibility shifts to git - > a.k.a., unintended consequences. It is important to note that my implementation does not engage in "moving comments around" at all. It merely supports adding comment suffixes to config lines *while they are created by git-config*. No after-the-fact comment manipulation is done. There are no multiline comments involved either, which I should have mentioned from the get-go. The limited scope of my proposal is deliberate, and aimed at avoiding possible problems based on the design alone. -Ralph
diff --git a/Documentation/git-config.txt b/Documentation/git-config.txt index dff39093b5e..ee8cd251b24 100644 --- a/Documentation/git-config.txt +++ b/Documentation/git-config.txt @@ -9,9 +9,9 @@ git-config - Get and set repository or global options SYNOPSIS -------- [verse] -'git config' [<file-option>] [--type=<type>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]] -'git config' [<file-option>] [--type=<type>] --add <name> <value> -'git config' [<file-option>] [--type=<type>] [--fixed-value] --replace-all <name> <value> [<value-pattern>] +'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] [--show-origin] [--show-scope] [-z|--null] <name> [<value> [<value-pattern>]] +'git config' [<file-option>] [--type=<type>] [--comment=<value>] --add <name> <value> +'git config' [<file-option>] [--type=<type>] [--comment=<value>] [--fixed-value] --replace-all <name> <value> [<value-pattern>] 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get <name> [<value-pattern>] 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] --get-all <name> [<value-pattern>] 'git config' [<file-option>] [--type=<type>] [--show-origin] [--show-scope] [-z|--null] [--fixed-value] [--name-only] --get-regexp <name-regex> [<value-pattern>] @@ -87,6 +87,10 @@ OPTIONS values. This is the same as providing '^$' as the `value-pattern` in `--replace-all`. +--comment:: + Append a comment to new or modified lines. A '#' character + will be automatically prepended to the value. + --get:: Get the value for a given key (optionally filtered by a regex matching the value). Returns error code 1 if the key was not diff --git a/builtin/config.c b/builtin/config.c index b55bfae7d66..2aab3c0baf3 100644 --- a/builtin/config.c +++ b/builtin/config.c @@ -44,6 +44,7 @@ static struct config_options config_options; static int show_origin; static int show_scope; static int fixed_value; +static const char *comment; #define ACTION_GET (1<<0) #define ACTION_GET_ALL (1<<1) @@ -173,6 +174,7 @@ static struct option builtin_config_options[] = { OPT_BOOL(0, "show-origin", &show_origin, N_("show origin of config (file, standard input, blob, command line)")), OPT_BOOL(0, "show-scope", &show_scope, N_("show scope of config (worktree, local, global, system, command)")), OPT_STRING(0, "default", &default_value, N_("value"), N_("with --get, use default value when missing entry")), + OPT_STRING(0, "comment", &comment, N_("value"), N_("human-readable comment string (# will be prepended automatically)")), OPT_END(), }; @@ -797,6 +799,11 @@ int cmd_config(int argc, const char **argv, const char *prefix) usage_builtin_config(); } + if (comment && !(actions & (ACTION_ADD|ACTION_SET|ACTION_SET_ALL|ACTION_REPLACE_ALL))) { + error(_("--comment is only applicable to add/set/replace operations")); + usage_builtin_config(); + } + /* check usage of --fixed-value */ if (fixed_value) { int allowed_usage = 0; @@ -880,7 +887,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) check_write(); check_argc(argc, 2, 2); value = normalize_value(argv[0], argv[1], &default_kvi); - ret = git_config_set_in_file_gently(given_config_source.file, argv[0], value); + ret = git_config_set_in_file_gently(given_config_source.file, argv[0], comment, value); if (ret == CONFIG_NOTHING_SET) error(_("cannot overwrite multiple values with a single value\n" " Use a regexp, --add or --replace-all to change %s."), argv[0]); @@ -891,7 +898,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) value = normalize_value(argv[0], argv[1], &default_kvi); ret = git_config_set_multivar_in_file_gently(given_config_source.file, argv[0], value, argv[2], - flags); + comment, flags); } else if (actions == ACTION_ADD) { check_write(); @@ -900,7 +907,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) ret = git_config_set_multivar_in_file_gently(given_config_source.file, argv[0], value, CONFIG_REGEX_NONE, - flags); + comment, flags); } else if (actions == ACTION_REPLACE_ALL) { check_write(); @@ -908,7 +915,7 @@ int cmd_config(int argc, const char **argv, const char *prefix) value = normalize_value(argv[0], argv[1], &default_kvi); ret = git_config_set_multivar_in_file_gently(given_config_source.file, argv[0], value, argv[2], - flags | CONFIG_FLAGS_MULTI_REPLACE); + comment, flags | CONFIG_FLAGS_MULTI_REPLACE); } else if (actions == ACTION_GET) { check_argc(argc, 1, 2); @@ -936,17 +943,17 @@ int cmd_config(int argc, const char **argv, const char *prefix) if (argc == 2) return git_config_set_multivar_in_file_gently(given_config_source.file, argv[0], NULL, argv[1], - flags); + NULL, flags); else return git_config_set_in_file_gently(given_config_source.file, - argv[0], NULL); + argv[0], NULL, NULL); } else if (actions == ACTION_UNSET_ALL) { check_write(); check_argc(argc, 1, 2); return git_config_set_multivar_in_file_gently(given_config_source.file, argv[0], NULL, argv[1], - flags | CONFIG_FLAGS_MULTI_REPLACE); + NULL, flags | CONFIG_FLAGS_MULTI_REPLACE); } else if (actions == ACTION_RENAME_SECTION) { check_write(); diff --git a/builtin/gc.c b/builtin/gc.c index cb80ced6cb5..342907f7bdb 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -1553,7 +1553,7 @@ static int maintenance_register(int argc, const char **argv, const char *prefix) die(_("$HOME not set")); rc = git_config_set_multivar_in_file_gently( config_file, "maintenance.repo", maintpath, - CONFIG_REGEX_NONE, 0); + CONFIG_REGEX_NONE, NULL, 0); free(global_config_file); if (rc) @@ -1620,7 +1620,7 @@ static int maintenance_unregister(int argc, const char **argv, const char *prefi if (!config_file) die(_("$HOME not set")); rc = git_config_set_multivar_in_file_gently( - config_file, key, NULL, maintpath, + config_file, key, NULL, maintpath, NULL, CONFIG_FLAGS_MULTI_REPLACE | CONFIG_FLAGS_FIXED_VALUE); free(global_config_file); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index fda50f2af1e..e4e18adb575 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -1283,7 +1283,7 @@ static void sync_submodule(const char *path, const char *prefix, submodule_to_gitdir(&sb, path); strbuf_addstr(&sb, "/config"); - if (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url)) + if (git_config_set_in_file_gently(sb.buf, remote_key, NULL, sub_origin_url)) die(_("failed to update remote for submodule '%s'"), path); diff --git a/builtin/worktree.c b/builtin/worktree.c index 9c76b62b02d..a20cc8820e5 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -365,12 +365,12 @@ static void copy_filtered_worktree_config(const char *worktree_git_dir) if (!git_configset_get_bool(&cs, "core.bare", &bare) && bare && git_config_set_multivar_in_file_gently( - to_file, "core.bare", NULL, "true", 0)) + to_file, "core.bare", NULL, "true", NULL, 0)) error(_("failed to unset '%s' in '%s'"), "core.bare", to_file); if (!git_configset_get(&cs, "core.worktree") && git_config_set_in_file_gently(to_file, - "core.worktree", NULL)) + "core.worktree", NULL, NULL)) error(_("failed to unset '%s' in '%s'"), "core.worktree", to_file); diff --git a/config.c b/config.c index 3cfeb3d8bd9..a22594eabd9 100644 --- a/config.c +++ b/config.c @@ -3001,6 +3001,7 @@ static ssize_t write_section(int fd, const char *key, } static ssize_t write_pair(int fd, const char *key, const char *value, + const char *comment, const struct config_store_data *store) { int i; @@ -3041,7 +3042,10 @@ static ssize_t write_pair(int fd, const char *key, const char *value, strbuf_addch(&sb, value[i]); break; } - strbuf_addf(&sb, "%s\n", quote); + if (comment) + strbuf_addf(&sb, "%s #%s\n", quote, comment); + else + strbuf_addf(&sb, "%s\n", quote); ret = write_in_full(fd, sb.buf, sb.len); strbuf_release(&sb); @@ -3130,9 +3134,9 @@ static void maybe_remove_section(struct config_store_data *store, } int git_config_set_in_file_gently(const char *config_filename, - const char *key, const char *value) + const char *key, const char *comment, const char *value) { - return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, 0); + return git_config_set_multivar_in_file_gently(config_filename, key, value, NULL, comment, 0); } void git_config_set_in_file(const char *config_filename, @@ -3153,7 +3157,7 @@ int repo_config_set_worktree_gently(struct repository *r, if (r->repository_format_worktree_config) { char *file = repo_git_path(r, "config.worktree"); int ret = git_config_set_multivar_in_file_gently( - file, key, value, NULL, 0); + file, key, value, NULL, NULL, 0); free(file); return ret; } @@ -3195,6 +3199,7 @@ void git_config_set(const char *key, const char *value) int git_config_set_multivar_in_file_gently(const char *config_filename, const char *key, const char *value, const char *value_pattern, + const char *comment, unsigned flags) { int fd = -1, in_fd = -1; @@ -3245,7 +3250,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename, free(store.key); store.key = xstrdup(key); if (write_section(fd, key, &store) < 0 || - write_pair(fd, key, value, &store) < 0) + write_pair(fd, key, value, comment, &store) < 0) goto write_err_out; } else { struct stat st; @@ -3399,7 +3404,7 @@ int git_config_set_multivar_in_file_gently(const char *config_filename, if (write_section(fd, key, &store) < 0) goto write_err_out; } - if (write_pair(fd, key, value, &store) < 0) + if (write_pair(fd, key, value, comment, &store) < 0) goto write_err_out; } @@ -3444,7 +3449,7 @@ void git_config_set_multivar_in_file(const char *config_filename, const char *value_pattern, unsigned flags) { if (!git_config_set_multivar_in_file_gently(config_filename, key, value, - value_pattern, flags)) + value_pattern, NULL, flags)) return; if (value) die(_("could not set '%s' to '%s'"), key, value); @@ -3467,7 +3472,7 @@ int repo_config_set_multivar_gently(struct repository *r, const char *key, int res = git_config_set_multivar_in_file_gently(file, key, value, value_pattern, - flags); + NULL, flags); free(file); return res; } diff --git a/config.h b/config.h index 5dba984f770..a85a8271696 100644 --- a/config.h +++ b/config.h @@ -290,7 +290,7 @@ int git_config_pathname(const char **, const char *, const char *); int git_config_expiry_date(timestamp_t *, const char *, const char *); int git_config_color(char *, const char *, const char *); -int git_config_set_in_file_gently(const char *, const char *, const char *); +int git_config_set_in_file_gently(const char *, const char *, const char *, const char *); /** * write config values to a specific config file, takes a key/value pair as @@ -336,7 +336,7 @@ int git_config_parse_key(const char *, char **, size_t *); int git_config_set_multivar_gently(const char *, const char *, const char *, unsigned); void git_config_set_multivar(const char *, const char *, const char *, unsigned); int repo_config_set_multivar_gently(struct repository *, const char *, const char *, const char *, unsigned); -int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, unsigned); +int git_config_set_multivar_in_file_gently(const char *, const char *, const char *, const char *, const char *, unsigned); /** * takes four parameters: diff --git a/sequencer.c b/sequencer.c index f49a871ac06..4c91ca5a844 100644 --- a/sequencer.c +++ b/sequencer.c @@ -3460,54 +3460,54 @@ static int save_opts(struct replay_opts *opts) if (opts->no_commit) res |= git_config_set_in_file_gently(opts_file, - "options.no-commit", "true"); + "options.no-commit", NULL, "true"); if (opts->edit >= 0) - res |= git_config_set_in_file_gently(opts_file, "options.edit", + res |= git_config_set_in_file_gently(opts_file, "options.edit", NULL, opts->edit ? "true" : "false"); if (opts->allow_empty) res |= git_config_set_in_file_gently(opts_file, - "options.allow-empty", "true"); + "options.allow-empty", NULL, "true"); if (opts->allow_empty_message) res |= git_config_set_in_file_gently(opts_file, - "options.allow-empty-message", "true"); + "options.allow-empty-message", NULL, "true"); if (opts->keep_redundant_commits) res |= git_config_set_in_file_gently(opts_file, - "options.keep-redundant-commits", "true"); + "options.keep-redundant-commits", NULL, "true"); if (opts->signoff) res |= git_config_set_in_file_gently(opts_file, - "options.signoff", "true"); + "options.signoff", NULL, "true"); if (opts->record_origin) res |= git_config_set_in_file_gently(opts_file, - "options.record-origin", "true"); + "options.record-origin", NULL, "true"); if (opts->allow_ff) res |= git_config_set_in_file_gently(opts_file, - "options.allow-ff", "true"); + "options.allow-ff", NULL, "true"); if (opts->mainline) { struct strbuf buf = STRBUF_INIT; strbuf_addf(&buf, "%d", opts->mainline); res |= git_config_set_in_file_gently(opts_file, - "options.mainline", buf.buf); + "options.mainline", NULL, buf.buf); strbuf_release(&buf); } if (opts->strategy) res |= git_config_set_in_file_gently(opts_file, - "options.strategy", opts->strategy); + "options.strategy", NULL, opts->strategy); if (opts->gpg_sign) res |= git_config_set_in_file_gently(opts_file, - "options.gpg-sign", opts->gpg_sign); + "options.gpg-sign", NULL, opts->gpg_sign); for (size_t i = 0; i < opts->xopts.nr; i++) res |= git_config_set_multivar_in_file_gently(opts_file, "options.strategy-option", - opts->xopts.v[i], "^$", 0); + opts->xopts.v[i], "^$", NULL, 0); if (opts->allow_rerere_auto) res |= git_config_set_in_file_gently(opts_file, - "options.allow-rerere-auto", + "options.allow-rerere-auto", NULL, opts->allow_rerere_auto == RERERE_AUTOUPDATE ? "true" : "false"); if (opts->explicit_cleanup) res |= git_config_set_in_file_gently(opts_file, - "options.default-msg-cleanup", + "options.default-msg-cleanup", NULL, describe_cleanup_mode(opts->default_msg_cleanup)); return res; } diff --git a/submodule-config.c b/submodule-config.c index 54130f6a385..11428b4adad 100644 --- a/submodule-config.c +++ b/submodule-config.c @@ -978,7 +978,7 @@ int config_set_in_gitmodules_file_gently(const char *key, const char *value) { int ret; - ret = git_config_set_in_file_gently(GITMODULES_FILE, key, value); + ret = git_config_set_in_file_gently(GITMODULES_FILE, key, NULL, value); if (ret < 0) /* Maybe the user already did that, don't error out here */ warning(_("Could not update .gitmodules entry %s"), key); diff --git a/submodule.c b/submodule.c index 213da79f661..86630932d09 100644 --- a/submodule.c +++ b/submodule.c @@ -2052,7 +2052,7 @@ void submodule_unset_core_worktree(const struct submodule *sub) submodule_name_to_gitdir(&config_path, the_repository, sub->name); strbuf_addstr(&config_path, "/config"); - if (git_config_set_in_file_gently(config_path.buf, "core.worktree", NULL)) + if (git_config_set_in_file_gently(config_path.buf, "core.worktree", NULL, NULL)) warning(_("Could not unset core.worktree setting in submodule '%s'"), sub->path); diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 31c38786870..daddaedd23c 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh @@ -69,13 +69,18 @@ test_expect_success 'replace with non-match (actually matching)' ' cat > expect << EOF [section] - penguin = very blue Movie = BadPhysics UPPERCASE = true - penguin = kingpin + penguin = gentoo #Pygoscelis papua + disposition = peckish #find fish [Sections] WhatEver = Second EOF +test_expect_success 'append comments' ' + git config --replace-all --comment="Pygoscelis papua" section.penguin gentoo && + git config --comment="find fish" section.disposition peckish && + test_cmp expect .git/config +' test_expect_success 'non-match result' 'test_cmp expect .git/config' diff --git a/worktree.c b/worktree.c index b02a05a74a3..cf5eea8c931 100644 --- a/worktree.c +++ b/worktree.c @@ -807,9 +807,9 @@ int should_prune_worktree(const char *id, struct strbuf *reason, char **wtpath, static int move_config_setting(const char *key, const char *value, const char *from_file, const char *to_file) { - if (git_config_set_in_file_gently(to_file, key, value)) + if (git_config_set_in_file_gently(to_file, key, NULL, value)) return error(_("unable to set %s in '%s'"), key, to_file); - if (git_config_set_in_file_gently(from_file, key, NULL)) + if (git_config_set_in_file_gently(from_file, key, NULL, NULL)) return error(_("unable to unset %s in '%s'"), key, from_file); return 0; }