Re: [PATCH 2/6] Teach remote.c about the remote.default configuration setting.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



marcnarc@xxxxxxxxxxx writes:

> From: Marc Branchaud <marcnarc@xxxxxxxxxxx>
>
> The code now has a default_remote_name and an effective_remote_name:
>  - default_remote_name is set by remote.default in the config, or is "origin"
>    if remote.default doesn't exist ("origin" was the fallback value before
>    this change).


>  - effective_remote_name is the name of the remote tracked by the current
>    branch, or is default_remote_name if the current branch doesn't have a
>    remote.

The explanation of the latter belongs to the previous step, I think.
I am not sure if "effective" is the best name for the concept the
above explains, though.

> @@ -390,6 +391,7 @@ static int handle_config(const char *key, const char *value, void *cb)
>  	}
>  	if (prefixcmp(key,  "remote."))
>  		return 0;
> +

Why?

>  	name = key + 7;
> @@ -671,6 +680,18 @@ static int valid_remote_nick(const char *name)
>  	return !strchr(name, '/'); /* no slash */
>  }
>  
> +const char *remote_get_default_name()

const char *remote_get_default_name(void)

> +{
> +	read_config();
> +	return default_remote_name;
> +}

Hrmph.  I am too lazy to read outside the context of your patch to
make sure, but isn't the root cause of the problem that when we try
to find which remote the current branch is configured to interact
with, we grab branch->remote_name (and this is done by calling
git_config() to open and read the configuration file once already)
and if it is empty we default to "origin"?  Wouldn't the callback
function that is used for that invocation of git_config() a much
better place to set "default_remote_name" variable, instead of
having us to read the entire configuration file one more time only
to get the value of this variable?

> +int remote_count()

int remote_count(void)

> +{
> +	read_config();
> +	return remotes_nr;
> +}

Likewise.  Especially it is unclear who benefits from the function
until a new caller is introduced.  I would prefer not to see the
addition of this function in this patch.

>  struct remote *remote_get(const char *name)
>  {
>  	struct remote *ret;
> diff --git a/remote.h b/remote.h
> index 251d8fd..f9aac87 100644
> --- a/remote.h
> +++ b/remote.h
> @@ -52,6 +52,8 @@ struct remote {
>  
>  struct remote *remote_get(const char *name);
>  int remote_is_configured(const char *name);
> +const char *remote_get_default_name();
> +int remote_count();

const char *remote_get_default_name(void);
int remote_count(void);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]