Re: [PATCH v13 4/4] ls-remote: create '--sort' option

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Harald Nordgren <haraldnordgren@xxxxxxxxx> writes:

> Create a '--sort' option for ls-remote, based on the one from
> for-each-ref. This e.g. allows ref names to be sorted by version
> semantics, so that v1.2 is sorted before v1.10.
>
> Signed-off-by: Harald Nordgren <haraldnordgren@xxxxxxxxx>
> ---

Thanks.

> +--sort=<key>::
> +	Sort based on the key given. Prefix `-` to sort in descending order
> +	of the value. Supports "version:refname" or "v:refname" (tag names
> +	are treated as versions). The "version:refname" sort order can also
> +	be affected by the "versionsort.suffix" configuration variable.
> +	See linkgit:git-for-each-ref[1] for more sort options, but be aware
> +	that because `ls-remote` deals only with remotes, keys like
> +	`committerdate` that requires access to the objects themselves will
> +	not work for refs whose objects have not yet been fetched from the
> +	remote.

With the update since v12, I think "because `ls-remote` deals only
with remotes," can be dropped entirely, and still convey what needs
to be told: "Be aware some keys that needs access to objects that
are not here won't work".

Instead, it is probably a better idea to spend that half-line worth
of characters to describe in what way they do not work (do they try
to deref a NULL pointer and dump core?  do they notice we need
missing objects and give an error?).

> diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
> index 540d56429..b5ca67167 100644
> --- a/builtin/ls-remote.c
> +++ b/builtin/ls-remote.c

As I said earlier, let's keep these extra UNLEAK() near early
return; they point developers at the places that needs future work.

Thanks.



diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index b5ca67167d..d3851074c2 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -98,6 +98,7 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 
 	if (get_url) {
 		printf("%s\n", *remote->url);
+		UNLEAK(sorting);
 		return 0;
 	}
 
@@ -106,8 +107,10 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 		transport_set_option(transport, TRANS_OPT_UPLOADPACK, uploadpack);
 
 	ref = transport_get_remote_refs(transport);
-	if (transport_disconnect(transport))
+	if (transport_disconnect(transport)) {
+		UNLEAK(sorting);
 		return 1;
+	}
 
 	if (!dest && !quiet)
 		fprintf(stderr, "From %s\n", *remote->url);





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux