Re: [PATCH v5 2/2] worktree: add 'list' command
On Sat, Aug 22, 2015 at 11:51 PM, Michael Rappazzo wrote: > 'git worktree list' uses the for_each_worktree function to iterate, > and outputs in the format: ' ()' > > Signed-off-by: Michael Rappazzo > --- > Documentation/git-worktree.txt | 11 +- > builtin/worktree.c | 55 > t/t2027-worktree-list.sh | 81 > ++ > 3 files changed, 146 insertions(+), 1 deletion(-) > create mode 100755 t/t2027-worktree-list.sh > > diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt > index fb68156..e953b4e 100644 > --- a/Documentation/git-worktree.txt > +++ b/Documentation/git-worktree.txt > @@ -11,6 +11,7 @@ SYNOPSIS > [verse] > 'git worktree add' [-f] [--detach] [-b ] [] > 'git worktree prune' [-n] [-v] [--expire ] > +'git worktree list' [--path-only] > > DESCRIPTION > --- > @@ -59,6 +60,12 @@ prune:: > > Prune working tree information in $GIT_DIR/worktrees. > > +list:: > + > +List the main worktree followed by all of the linked worktrees. The default > +format of the list includes the full path to the worktree and the branch or > +revision that the head of that worktree is currently pointing to. Maybe just "and the branch or revision currently checked out in that worktree."? -- Mikael Magnusson -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] worktree: add 'list' command
On Mon, Aug 24, 2015 at 2:05 PM, Junio C Hamano wrote:
> Michael Rappazzo writes:
>> + strbuf_strip_suffix(&head_ref, "\n");
>> +
>> + if (starts_with(head_ref.buf, ref_prefix)) {
>> + /* branch checked out */
>> + strbuf_remove(&head_ref, 0, strlen(ref_prefix));
>> + /* } else {
>> + * headless -- no-op
>> + */
>> + }
>> + printf("%s (%s)\n", path, head_ref.buf);
>
> Is this new command meant to be a Porcelain? This would not work as
> a plumbing that produces a machine-parseable stable output.
>
> I am not saying that it _should_; I do not know if we even need a
> 'list' command that is driven from an end-user script that gives
> anything more than "where the work trees are".
>
> My inclination is to suggest dropping the "which branch" code
> altogether and only give "path_only" behaviour.
The "which branch" was probably added in response to this [1] review,
which suggested that at some point, we might want to provide the user
with interesting information about each worktree, such as
branch/detached head, tag, locked status (plus lock reason and whether
currently accessible), prune-able status (plus reason). This could
optionally be controlled by --verbose or some other extended
formatting option.
The same review also suggested a --porcelain option for script writers.
[1]: http://article.gmane.org/gmane.comp.version-control.git/275528
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 2/2] worktree: add 'list' command
Michael Rappazzo writes:
> +static int print_worktree_details(const char *path, const char *git_dir,
> void *cb_data)
> +{
> + struct strbuf head_file = STRBUF_INIT;
> + struct strbuf head_ref = STRBUF_INIT;
> + struct stat st;
> + struct list_opts *opts = cb_data;
> + const char *ref_prefix = "ref: refs/heads/";
> +
> + strbuf_addf(&head_file, "%s/HEAD", git_dir);
> + if (!opts->path_only && !stat(head_file.buf, &st)) {
> + strbuf_read_file(&head_ref, head_file.buf, st.st_size);
This does not work for traditional "symlinked HEAD", no?
I'd prefer to see the callback functions of for_each_worktree() not
to duplicate the logic we already have elsewhere in the system.
Such an incomplete attempt to duplication (as we see here) will lead
to unnecessary bugs (as we see here).
Conceptually, for-each-worktree should give us the worktree root
(i.e. equivalent to $GIT_WORK_TREE in the pre-multi-worktree world)
and the git directory (i.e. equivalent to $GIT_DIR in the
pre-multi-worktree world), and the callbacks should be able to do an
equivalent of
system("git --work-tree=$GIT_WORK_TREE --git-dir=$GIT_DIR cmd")
where in this particular case "cmd" may be "symbolic-ref HEAD" or
something, no?
> + strbuf_strip_suffix(&head_ref, "\n");
> +
> + if (starts_with(head_ref.buf, ref_prefix)) {
> + /* branch checked out */
> + strbuf_remove(&head_ref, 0, strlen(ref_prefix));
> + /* } else {
> + * headless -- no-op
> + */
> + }
> + printf("%s (%s)\n", path, head_ref.buf);
Is this new command meant to be a Porcelain? This would not work as
a plumbing that produces a machine-parseable stable output.
I am not saying that it _should_; I do not know if we even need a
'list' command that is driven from an end-user script that gives
anything more than "where the work trees are".
My inclination is to suggest dropping the "which branch" code
altogether and only give "path_only" behaviour.
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/2] worktree: add 'list' command
'git worktree list' uses the for_each_worktree function to iterate,
and outputs in the format: ' ()'
Signed-off-by: Michael Rappazzo
---
Documentation/git-worktree.txt | 11 +-
builtin/worktree.c | 55
t/t2027-worktree-list.sh | 81 ++
3 files changed, 146 insertions(+), 1 deletion(-)
create mode 100755 t/t2027-worktree-list.sh
diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
index fb68156..e953b4e 100644
--- a/Documentation/git-worktree.txt
+++ b/Documentation/git-worktree.txt
@@ -11,6 +11,7 @@ SYNOPSIS
[verse]
'git worktree add' [-f] [--detach] [-b ] []
'git worktree prune' [-n] [-v] [--expire ]
+'git worktree list' [--path-only]
DESCRIPTION
---
@@ -59,6 +60,12 @@ prune::
Prune working tree information in $GIT_DIR/worktrees.
+list::
+
+List the main worktree followed by all of the linked worktrees. The default
+format of the list includes the full path to the worktree and the branch or
+revision that the head of that worktree is currently pointing to.
+
OPTIONS
---
@@ -93,6 +100,9 @@ OPTIONS
--expire ::
With `prune`, only expire unused working trees older than .
+--path-only
+ With `list`, only show the worktree path
+
DETAILS
---
Each linked working tree has a private sub-directory in the repository's
@@ -167,7 +177,6 @@ performed manually, such as:
- `remove` to remove a linked working tree and its administrative files (and
warn if the working tree is dirty)
- `mv` to move or rename a working tree and update its administrative files
-- `list` to list linked working trees
- `lock` to prevent automatic pruning of administrative files (for instance,
for a working tree on a portable device)
diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7b3cb96..673f292 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -12,6 +12,7 @@
static const char * const worktree_usage[] = {
N_("git worktree add [] "),
N_("git worktree prune []"),
+ N_("git worktree list []"),
NULL
};
@@ -442,6 +443,58 @@ done:
return ret;
}
+/* list callback data */
+struct list_opts {
+ int path_only;
+};
+
+static int print_worktree_details(const char *path, const char *git_dir, void
*cb_data)
+{
+ struct strbuf head_file = STRBUF_INIT;
+ struct strbuf head_ref = STRBUF_INIT;
+ struct stat st;
+ struct list_opts *opts = cb_data;
+ const char *ref_prefix = "ref: refs/heads/";
+
+ strbuf_addf(&head_file, "%s/HEAD", git_dir);
+ if (!opts->path_only && !stat(head_file.buf, &st)) {
+ strbuf_read_file(&head_ref, head_file.buf, st.st_size);
+ strbuf_strip_suffix(&head_ref, "\n");
+
+ if (starts_with(head_ref.buf, ref_prefix)) {
+ /* branch checked out */
+ strbuf_remove(&head_ref, 0, strlen(ref_prefix));
+ /* } else {
+* headless -- no-op
+*/
+ }
+ printf("%s (%s)\n", path, head_ref.buf);
+ } else {
+ printf("%s\n", path);
+ }
+
+ strbuf_release(&head_ref);
+ strbuf_release(&head_file);
+ return 0;
+}
+
+static int list(int ac, const char **av, const char *prefix)
+{
+ struct list_opts opts;
+ struct option options[] = {
+ OPT_BOOL(0, "path-only", &opts.path_only, N_("only show the
path of the worktree")),
+ OPT_END()
+ };
+
+ opts.path_only = 0;
+
+ ac = parse_options(ac, av, prefix, options, worktree_usage, 0);
+ if (ac)
+ usage_with_options(worktree_usage, options);
+
+ return for_each_worktree(&print_worktree_details, &opts);
+}
+
int cmd_worktree(int ac, const char **av, const char *prefix)
{
struct option options[] = {
@@ -454,5 +507,7 @@ int cmd_worktree(int ac, const char **av, const char
*prefix)
return add(ac - 1, av + 1, prefix);
if (!strcmp(av[1], "prune"))
return prune(ac - 1, av + 1, prefix);
+ if (!strcmp(av[1], "list"))
+ return list(ac - 1, av + 1, prefix);
usage_with_options(worktree_usage, options);
}
diff --git a/t/t2027-worktree-list.sh b/t/t2027-worktree-list.sh
new file mode 100755
index 000..66a635a
--- /dev/null
+++ b/t/t2027-worktree-list.sh
@@ -0,0 +1,81 @@
+#!/bin/sh
+
+test_description='test git worktree list'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+ test_commit init
+'
+
+test_expect_success '"list" all worktrees from main' '
+ echo "$(git rev-parse --show-toplevel) ($(git symbolic-ref --short
HEAD))" >expect &&
+ git worktree add --detach here master &&
+ echo "$(git -C here rev-parse --show-toplevel) ($(git -C here
rev-parse HEAD))" >>expect &&
+ git worktree list >actual &&
+ test_cmp expect actual &&
