Re: [PATCH v7 3/3] worktree: add 'list' command
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Sep 11, 2015, 22:02 UTC
- Message-ID
- <xmqq7fnwzkbe.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1441402769-35897-4-git-send-email-rappazzo@gmail.com>
Michael Rappazzo <rappazzo@gmail.com> writes:
Show 37 quoted lines
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 71bb770..a0c0fe8 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -8,10 +8,13 @@
> #include "run-command.h"
> #include "sigchain.h"
> #include "refs.h"
> +#include "utf8.h"
> +#include "worktree.h"
>
> static const char * const worktree_usage[] = {
> N_("git worktree add [<options>] <path> <branch>"),
> N_("git worktree prune [<options>]"),
> + N_("git worktree list [<options>]"),
> NULL
> };
>
> @@ -359,6 +362,64 @@ static int add(int ac, const char **av, const char *prefix)
> return add_worktree(path, branch, &opts);
> }
>
> +static int list(int ac, const char **av, const char *prefix)
> +{
> + int no_bare = 0;
> +
> + struct option options[] = {
> + OPT_BOOL(0, "skip-bare", &no_bare, N_("exclude bare worktrees from the list")),
> + OPT__VERBOSE(&verbose, N_("include more worktree details")),
> + OPT_END()
> + };
> +
> + ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
> + if (ac)
> + usage_with_options(worktree_usage, options);
> + else {
> + struct worktree_list *worktree_list = get_worktree_list();Call that variable just "list"; you are not dealing with any other kind of list in this code anyway.
Show 10 quoted lines
> + struct worktree_list *orig = worktree_list;
> + struct strbuf sb = STRBUF_INIT;
> + int path_maxlen = 0;
> +
> + if (verbose) {
> + while (worktree_list) {
> + int cur_len = strlen(worktree_list->worktree->path);
> + if (cur_len > path_maxlen)
> + path_maxlen = cur_len;
> + worktree_list = worktree_list->next ? worktree_list->next : NULL;Ehh, (A ? A : NULL), when A is a pointer value, is equivalent to A, no?
struct worktree_list *orig = get_worktree_list(); struct worktree_list *list;
for (list = orig; list; list = list->next) {
struct worktree *worktree = list->worktree;
int len = strlen(worktree->path);
if (maxlen < len)
maxlen = len;
}It seems that "do we really need callback interface?" suggestion really made the code simple and straight-forward.
Show 23 quoted lines
> + while (worktree_list) {
> + /* if this work tree is not bare OR if bare repos are to be shown (default) */
> + if (!worktree_list->worktree->is_bare || !no_bare) {
> + strbuf_reset(&sb);
> + if (!verbose)
> + strbuf_addstr(&sb, worktree_list->worktree->path);
> + else {
> + int cur_len = strlen(worktree_list->worktree->path);
> + int utf8_adj = cur_len - utf8_strwidth(worktree_list->worktree->path);
> + strbuf_addf(&sb, "%-*s ", 1 + path_maxlen + utf8_adj, worktree_list->worktree->path);
> + if (worktree_list->worktree->is_bare)
> + strbuf_addstr(&sb, "(bare)");
> + else {
> + strbuf_addf(&sb, "%s ", find_unique_abbrev(worktree_list->worktree->head_sha1, DEFAULT_ABBREV));
> + if (!worktree_list->worktree->is_detached)
> + strbuf_addf(&sb, "[%s]", shorten_unambiguous_ref(worktree_list->worktree->head_ref, 0));
> + else
> + strbuf_addstr(&sb, "(detached HEAD)");
> + }
> + }
> + printf("%s\n", sb.buf);
> + }
> + worktree_list = worktree_list->next ? worktree_list->next : NULL;Would it help reduce the indentation level if you inroduced a helper function that takes a single worktree object and does all the above?
for (list = orig; list; list = list->next) {
struct worktree *worktree = list->worktree;
if (!no_bare || !worktree->is_bare)
show_worktree(worktree);
}or something?