Re: [PATCH v3] worktree: add 'list' command

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 11, 2015, 15:46 UTC
Message-ID: <xmqq37zpu8tv.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CANoM8SWeqxD2vWLQmEfxxxn8Dz4yPfjGOoOH=Azn1A3So+wz2Q@mail.gmail.com>

Mike Rappazzo <rappazzo@gmail.com> writes:

Show 5 quoted lines

>>> +     int is_bare = is_bare_repository();
>>
>> Please do not introduce decl-after-stmt.
>
> Since I reused this value below, I thought it would be acceptable.

Use of a new variable is fine. "Do not declare one in a block after you already wrote statement" is what "decl-after-stmt not allowed" means. In your patch:

+static int list(int ac, const char **av, const char *prefix) +{ + int main_only = 0; + struct option options[] = { + OPT_BOOL(0, "main-only", &main_only, N_("only list the main worktree")), + OPT_END() + }; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac) + usage_with_options(worktree_usage, options); + + struct strbuf main_path = STRBUF_INIT; + const char* common_dir = get_git_common_dir(); + int is_bare = is_bare_repository();

Three variables, main_path, common_dir and is_bare are declared here after statements such as a call to parse_options(). Don't.

+static int list(int ac, const char **av, const char *prefix) +{ + int main_only = 0; + struct strbuf main_path = STRBUF_INIT; + const char *common_dir; + int is_bare; + struct option options[] = { + OPT_BOOL(0, "main-only", &main_only, N_("only list the main worktree")), + OPT_END() + }; + + ac = parse_options(ac, av, prefix, options, worktree_usage, 0); + if (ac) + usage_with_options(worktree_usage, options); + + common_dir = get_git_common_dir(); + int is_bare = is_bare_repository();