Re: [PATCH] branch: advise about ref syntax rules

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

Kristoffer Haugsbakk <code@khaugsbakk.name> writes:

> Notes (series):
>     Hopefully I am using `advice.h` correctly here.

Let's see.

> -	if (strbuf_check_branch_ref(ref, name))
> -		die(_("'%s' is not a valid branch name"), name);
> +	if (strbuf_check_branch_ref(ref, name)) {
> +		error(_("'%s' is not a valid branch name"), name);
> +		advise(_("See `man git check-ref-format`"));
> +		exit(1);
> +	}

This will give the message with "hint:" prefix, which is a good
starting point.

The message is given unconditionally, without any way to turn it
off.  For those who ...

> git-branch(1) will error out if you give it a bad ref name. But the user
> might not understand why or what part of the name is illegal.

... do not understand why, it is helpful, but once they learned, it
is one extra line of unwanted text.  If you want to give it a way to
squelch, see the comment before where enum advice_type is declared
in advice.h header file.  The callsites would become something like

	advise_if_enabled(ADVICE_VALID_REF_NAME,
		_("See `man git check-ref-format` for valid refname syntax."));

Another thing is that rewriting die() into error() + advice() +
manual exit() is an anti-pattern these days.

	int code = die_message(_("'%s' is not a valid branch name"), name);
	advice_if_enabled(...); /* see above */
	exit(code);

In the same source file, you will find an existing example to mimic.

Thanks.