Re: [PATCH v7 3/3] worktree: add 'list' command
From: Junio C Hamano <hidden>
Date: 2016-06-15 23:06:31
Michael Rappazzo [off-list ref] writes:
quoted hunk
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.
+ 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.
+ 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?