Xiaolong Ye <xiaolong...@intel.com> writes:
> +static int config_base_commit;
This variable is used as a simple boolean whose name is overly broad
(if it were named "config_base_auto" this complaint would not
apply). If you envision possible future enhancements for this
configuration variable, "int config_base_commit" might make sense
but I don't think of anything offhand that would be happy with
"int".
> @@ -786,6 +787,12 @@ static int git_format_config(const char *var, const char
> *value, void *cb)
> }
> if (!strcmp(var, "format.outputdirectory"))
> return git_config_string(&config_output_directory, var, value);
> + if (!strcmp(var, "format.base")){
Style. s/)){/)) {/
> + if (value && !strcasecmp(value, "auto")) {
Does it make sense to allow "Auto" here? Given that the command
line parsing uses strcmp() to require "auto", I do not think so.
> + config_base_commit = 1;
> + return 0;
> + }
When a value other than "auto" is given, is it sane to ignore them
without even warning?
I am wondering if this wants to be a format.useAutoBase boolean
variable.
> @@ -1215,7 +1222,12 @@ static void prepare_bases(struct base_tree_info *bases,
> DIFF_OPT_SET(&diffopt, RECURSIVE);
> diff_setup_done(&diffopt);
>
> - if (!strcmp(base_commit, "auto")) {
> + if (base_commit && strcmp(base_commit, "auto")) {
> + base = lookup_commit_reference_by_name(base_commit);
> + if (!base)
> + die(_("Unknown commit %s"), base_commit);
> + oidcpy(&bases->base_commit, &base->object.oid);
> + } else if ((base_commit && !strcmp(base_commit, "auto")) ||
> config_base_commit) {
It may be a poor design to teach prepare_bases() about "auto" thing.
Doesn't it belong to the caller? The caller used to say "If a base
is given, then call that function, by the way, the base must be a
concrete one", and with the new "auto" feature, the caller loosens
the last part of the statement and says "If a base is given, call
that function, but if it is specified as "auto", I'd have to compute
it for the user before doing so".
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
