On Wed, Jan 8, 2025 at 7:39 PM Eric Ju <eric.peijian@xxxxxxxxx> wrote:
>
> Since the `info` command in cat-file --batch-command prints object info
Nit: Everywhere in this commit message, it would be a bit clearer and
easier to read with:
s/cat-file --batch-command/`cat-file --batch-command`/
> for a given object, it is natural to add another command in cat-file
> --batch-command to print object info for a given object from a remote.
>
> Add `remote-object-info` to cat-file --batch-command.
s/`remote-object-info`/a new `remote-object-info` command/
[...]
> To summarize, `remote-object-info` gets object info from the remote and
> then loop through the object info passed in, printing the info.
s/loop/loops/
> +remote-object-info <remote> <object>...::
> + Print object info for object references `<object>` at specified
> + `<remote>` without downloading objects from the remote.
> + Error when the `object-info` capability is not supported by the server.
I think it's more grammatically correct to use "Error out when..." or
"Raise an error when..." than just "Error when..."
Also maybe: s/server/remote/
> + Error when no object references are provided.
Here also "Error out when..." or "Raise an error when..."
> + This command may be combined with `--buffer`.
[...]
> If no format is specified, the default format is `%(objectname)
> -%(objecttype) %(objectsize)`.
> +%(objecttype) %(objectsize)`, except for `remote-object-info` commands which use
> +`%(objectname) %(objectsize)` for now because "%(objecttype)" is not supported yet.
> +WARNING: When "%(objecttype)" is supported, the default format WILL be unified, so
> +DO NOT RELY on the current the default format to stay the same!!!
s/current the default/current default/
> CAVEATS
> -------
>
> +Note that since %(objecttype), %(objectsize:disk) and %(deltabase) are
> +currently not supported by the `remote-object-info` command, we will error
s/error/raise an error/
or maybe:
s/error and exit/error out/
> +and exit when they are in the format string.
s/are/appear/
> @@ -45,9 +48,12 @@ struct batch_options {
> char input_delim;
> char output_delim;
> const char *format;
> + int use_remote_info;
"unsigned int" might be a bit better for bool fields like this.
Actually it seems to me that this field is set to 0 and 1 in some
places but we never read it, so I wonder if it's actually useful.
> };
> @@ -579,6 +585,61 @@ static void batch_one_object(const char *obj_name,
> object_context_release(&ctx);
> }
>
> +static int get_remote_info(struct batch_options *opt, int argc, const char **argv)
> +{
> + int retval = 0;
> + struct remote *remote = NULL;
> + struct object_id oid;
> + struct string_list object_info_options = STRING_LIST_INIT_NODUP;
> + static struct transport *gtransport;
> +
> + /*
> + * Change the format to "%(objectname) %(objectsize)" when
> + * remote-object-info command is used. Once we start supporting objecttype
> + * the default format should change to DEFAULT_FORMAT
s/DEFAULT_FORMAT/DEFAULT_FORMAT./
> + */
> + if (!opt->format)
> + opt->format = "%(objectname) %(objectsize)";
> +
> + remote = remote_get(argv[0]);
> + if (!remote)
> + die(_("must supply valid remote when using remote-object-info"));
> +
> + oid_array_clear(&object_info_oids);
> + for (size_t i = 1; i < argc; i++) {
> + if (get_oid_hex(argv[i], &oid))
> + die(_("Not a valid object name %s"), argv[i]);
> + oid_array_append(&object_info_oids, &oid);
> + }
> + if (object_info_oids.nr == 0) {
> + die(_("remote-object-info requires objects"));
> + }
We prefer to drop '{' and '}' and use "!X" instead of "X == 0" when
possible, so:
if (!object_info_oids.nr)
die(_("remote-object-info requires objects"));
Thanks.
