Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"

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

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> +static char *get_head_description()
> +{
> +	struct stat st;
> +	struct strbuf sb = STRBUF_INIT;
> +	struct strbuf result = STRBUF_INIT;
> +	int bisect = 0;
> +	int ret;
> +	if (!stat(git_path("rebase-merge"), &st) && S_ISDIR(st.st_mode))
> +		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);

Hrmph.  Why isn't this checking if the file exists and then read it,
i.e.

	if (access(git_path("rebase-merge/head-name"), F_OK))
		ret = strbuf_read_file(&sb, git_path("rebase-merge/head-name"), 0);

It is not like you are creating this file and making sure leading
directories exist, so the sequence looks a bit strange.

> +	else if (!access(git_path("rebase-apply/rebasing"), F_OK))
> +		ret = strbuf_read_file(&sb, git_path("rebase-apply/head-name"), 0);
> +	else if (!access(git_path("BISECT_LOG"), F_OK)) {
> +		ret = strbuf_read_file(&sb, git_path("BISECT_START"), 0);
> +		bisect = 1;

And if the answer to the above question is "because if rebase-merge/
exists, with or without head-name, we know we are not bisecting",
then that may suggest that the structure of if/elseif cascade is
misdesigned.  Shouldn't the "bisect" boolean be an enum "what are we
doing" that is initialized to "I do not know" and each of these
if/elseif cascade set the state to it when they know what we are
doing, in order for this function to be longer-term maintainable?