Re: [PATCH v3] branch: show rebase/bisect info when possible instead of "(no branch)"
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 11, 2013, 19:13 UTC
- Message-ID
- <7vehgmmzop.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1360318171-17614-1-git-send-email-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 9 quoted lines
> +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.
Show 5 quoted lines
> + 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?