| Message ID | 20210407225608.14611-4-sorganov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | git log: configurable default format for merge diffs | expand |
On Thu, Apr 08, 2021 at 01:56:02AM +0300, Sergey Organov wrote: > New log.diffMerges configuration variable sets the format that > --diff-merges=def will be using. The default is "separate". > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > --- > builtin/log.c | 2 ++ > diff-merges.c | 11 +++++++++++ > diff-merges.h | 2 ++ > 3 files changed, 15 insertions(+) Please don't forget to document this new configuration variable.
On Thu, Apr 08, 2021 at 11:37:36PM +0200, SZEDER Gábor wrote: > On Thu, Apr 08, 2021 at 01:56:02AM +0300, Sergey Organov wrote: > > New log.diffMerges configuration variable sets the format that > > --diff-merges=def will be using. The default is "separate". > > > > Signed-off-by: Sergey Organov <sorganov@gmail.com> > > --- > > builtin/log.c | 2 ++ > > diff-merges.c | 11 +++++++++++ > > diff-merges.h | 2 ++ > > 3 files changed, 15 insertions(+) > > Please don't forget to document this new configuration variable. Oh, just noticed that you do document it in the last patch of the series, and, similarly, you add new options early in this patch series and add the corresponding documentation in the second to last patch. Please squash in those documentation updates into the corresponding patches.
SZEDER Gábor <szeder.dev@gmail.com> writes: > On Thu, Apr 08, 2021 at 11:37:36PM +0200, SZEDER Gábor wrote: >> On Thu, Apr 08, 2021 at 01:56:02AM +0300, Sergey Organov wrote: >> > New log.diffMerges configuration variable sets the format that >> > --diff-merges=def will be using. The default is "separate". >> > >> > Signed-off-by: Sergey Organov <sorganov@gmail.com> >> > --- >> > builtin/log.c | 2 ++ >> > diff-merges.c | 11 +++++++++++ >> > diff-merges.h | 2 ++ >> > 3 files changed, 15 insertions(+) >> >> Please don't forget to document this new configuration variable. > > Oh, just noticed that you do document it in the last patch of the > series, and, similarly, you add new options early in this patch series > and add the corresponding documentation in the second to last patch. > Please squash in those documentation updates into the corresponding > patches. Since any new topic that adds new configuration variable or update the description of an existing one would interact badly with the last step of Emily's es/config-hooks topic, b58f84c4 (docs: unify githooks and git-hook manpages, 2021-03-10), where the description of all the individual options are moved to a newly created file, and it is not practical to take all the new topics that touch the documentation for the configuration variables hostage to the topic that seems to be dormant for quite a while, I'll discard the last step from es/config-hooks topic for now. We really should get it moving soon (or discard and reboot it later---it is getting in the way for other topics to keep it in my tree, either way). Thanks.
SZEDER Gábor <szeder.dev@gmail.com> writes: > On Thu, Apr 08, 2021 at 11:37:36PM +0200, SZEDER Gábor wrote: >> On Thu, Apr 08, 2021 at 01:56:02AM +0300, Sergey Organov wrote: >> > New log.diffMerges configuration variable sets the format that >> > --diff-merges=def will be using. The default is "separate". >> > >> > Signed-off-by: Sergey Organov <sorganov@gmail.com> >> > --- >> > builtin/log.c | 2 ++ >> > diff-merges.c | 11 +++++++++++ >> > diff-merges.h | 2 ++ >> > 3 files changed, 15 insertions(+) >> >> Please don't forget to document this new configuration variable. > > Oh, just noticed that you do document it in the last patch of the > series, and, similarly, you add new options early in this patch series > and add the corresponding documentation in the second to last patch. > Please squash in those documentation updates into the corresponding > patches. Sorry, I fail to see how to do it that way, as documentation has mutual references between --diff-merges=def and log.diffMerges, that are introduced in different commits. That said, after squashing tests into corresponding code commits, that has been already requested, the documentation updates will be closer to the code commits. Is it OK with you then to leave documentation changes as separate commits? Also, Junio, please clarify what you prefer as maintainer, fine-grained commits, or squash everything even remotely relevant into a single one? I honestly fail to see where the preferred margin is, and start to get a feeling that I'd better squash everything together. Thanks, -- Sergey Organov
diff --git a/builtin/log.c b/builtin/log.c index 8acd285dafd8..6102893fccb9 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -481,6 +481,8 @@ static int git_log_config(const char *var, const char *value, void *cb) decoration_style = 0; /* maybe warn? */ return 0; } + if (!strcmp(var, "log.diffmerges")) + return diff_merges_config(value); if (!strcmp(var, "log.showroot")) { default_show_root = git_config_bool(var, value); return 0; diff --git a/diff-merges.c b/diff-merges.c index 93ede09fb36f..ca4d94a9039d 100644 --- a/diff-merges.c +++ b/diff-merges.c @@ -90,6 +90,17 @@ static void set_diff_merges(struct rev_info *revs, const char *optarg) * Public functions. They are in the order they are called. */ +int diff_merges_config(const char *value) +{ + diff_merges_setup_func_t func = func_by_opt(value); + + if (!func) + return -1; + + set_to_default = func; + return 0; +} + int diff_merges_parse_opts(struct rev_info *revs, const char **argv) { int argcount = 1; diff --git a/diff-merges.h b/diff-merges.h index 659467c99a4f..09d9a6c9a4fb 100644 --- a/diff-merges.h +++ b/diff-merges.h @@ -9,6 +9,8 @@ struct rev_info; +int diff_merges_config(const char *value); + int diff_merges_parse_opts(struct rev_info *revs, const char **argv); void diff_merges_suppress(struct rev_info *revs);
New log.diffMerges configuration variable sets the format that --diff-merges=def will be using. The default is "separate". Signed-off-by: Sergey Organov <sorganov@gmail.com> --- builtin/log.c | 2 ++ diff-merges.c | 11 +++++++++++ diff-merges.h | 2 ++ 3 files changed, 15 insertions(+)