Re: [PATCH] remote: add 'rm' subcommand

[Junio C Hamano <gitster@pobox.com>]

James Bowes <jbowes@dangerouslyinc.com> writes:

> Introduce git-remote rm <name> which will:
>  - Remove the remote config entry for <name>.
>  - Remove any config entries for tracking branches of <name>.
>  - Remove any stored remote branches of <name>.
>
> Signed-off-by: James Bowes <jbowes@dangerouslyinc.com>

Let's continue with this in 1.5.4 cycle.  Together with Dscho's
"--mirror" patch, this will make "remote" more complete.

> If there is any interest in this, I'll follow up later with additions
> to the docs.

This is not a good thing to say here.  If even the original
author cannot be bothered to perfect it unconditionally (iow
even when other people do not realize how useful it would be
initially and nobody seem to be interested), it does not help
convincing others that it is a good thing for the community to
take the patch.

> +sub rm_remote {
> +    my ($name) = @_;
> +	if (!exists $remote->{$name}) {
> +		print STDERR "No such remote $name\n";
> +		return;
> +	}
> +
> +	$git->command('config', '--remove-section', "remote.$name");
> +
> +	eval {
> +	    my @trackers = $git->command('config', '--get-regexp',
> +			'branch.*.remote', $name);

Is this correct, or should it be '^branch\..*\.remote$'

> +		for (@trackers) {
> +			/^branch\.(.*)?\.remote/;

And this one.  Why do we have '?' there?  Perhaps...

	if (/^branch\.(.*\)\.remote /) {
		$git->config('--unset', "branch.$1.remote");
		$git->config('--unset', "branch.$1.merge");
	} else {
        	die "Gaah, why $_ is not branch.<<name>>.remote???"
	}

We seem to have another subroutine to prune remote tracking
branches, which does it slightly differently.  Maybe we would
want to share code with that codepath?

Other than that, I think the patch is sane, with your later
"documentation patch".

We would want a handful tests, including ones to check error
conditions, such as trying to remove a remote that does not
exist.