Hi Stolee,
On Mon, 22 Jul 2019, Derrick Stolee via GitGitGadget wrote:
> From: Derrick Stolee <dstolee@xxxxxxxxxxxxx>
>
> When a repo has many commits, it is helpful to write and read the
> commit-graph file. Future changes to Git may include new config
> settings that are benefitial in this scenario.
s/benefitial/beneficial/
>
> Create the 'feature.manyCommits' config setting that changes the
> default values of 'core.commitGraph' and 'gc.writeCommitGraph' to
> true.
Great!
> diff --git a/repo-settings.c b/repo-settings.c
> index 13a9128f62..f328602fd7 100644
> --- a/repo-settings.c
> +++ b/repo-settings.c
> @@ -3,10 +3,17 @@
> #include "config.h"
> #include "repo-settings.h"
>
> +#define UPDATE_DEFAULT(s,v) do { if (s == -1) { s = v; } } while(0)
> +
> static int git_repo_config(const char *key, const char *value, void *cb)
> {
> struct repo_settings *rs = (struct repo_settings *)cb;
>
> + if (!strcmp(key, "feature.manycommits")) {
> + UPDATE_DEFAULT(rs->core_commit_graph, 1);
> + UPDATE_DEFAULT(rs->gc_write_commit_graph, 1);
> + return 0;
> + }
> if (!strcmp(key, "core.commitgraph")) {
> rs->core_commit_graph = git_config_bool(key, value);
> return 0;
Okay, this one is tricky. The behavior I would want is for
`feature.manycommits` to override the _default_. And if I set
`feature.manycommits = false` (e.g. via `git -c
feature.manycommits=false ...`), I would want the default to "go back".
So I'd really rather see this as
if (!repo_config_get_bool(r, "feature.manycommits", &b) && b) {
rs->core_commit_graph = 1;
rs->gc_write_commit_graph = 1;
}
[...]
repo_config_get_bool(r, "core.commitgraph", &rs->core_commit_graph);
I.e. override the default only if `feature.manyCommits` was true *in the
end*.
In any case, we really need to make sure to handle the `= false` case
correctly here. We might want to override the setting from the
command-line, or it might be set in `~/.gitconfig` and need to be
overridden in a local repository. We need to handle it.
Otherwise this looks good!
Dscho
