Re: [PATCH v2 02/18] cat-file: reuse struct ref_format

[Оля Тележная]

2018-01-16 0:37 GMT+03:00 Jeff King :
> On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote:
>
>> Start using ref_format struct instead of simple char*.
>> Need that for further reusing of formatting logic from ref-filter.
>
> OK, this makes sense (though again, at some point we want this to stop
> being a "ref_format" and just be a "format").
>
>>  struct batch_options {
>> + struct ref_format *format;
>
> Does this need to be a pointer? We can just store the ref_format inside
> the struct, right? And then...
>
>> @@ -557,7 +558,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
>> *prefix)
>>  {
>>   int opt = 0;
>>   const char *exp_type = NULL, *obj_name = NULL;
>> - struct batch_options batch = {0};
>> + struct ref_format format = REF_FORMAT_INIT;
>> + struct batch_options batch = {};
>>   int unknown_type = 0;
>
> ...here you would not need the extra local variable. You can initialize
> it like:
>
>   struct batch_options batch = { REF_FORMAT_INIT };
>
> -Peff

Thanks a lot!
Fixed, please check new version here: https://github.com/git/git/pull/450
If everything else is OK, I will send it to the mailing list.
As I said in other email threads, not sure that we need to include
last commit ("make valid_atom general again") into a new patch.

Thanks again,
Olga

Re: [PATCH v2 02/18] cat-file: reuse struct ref_format

[Jeff King]

On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote:

> Start using ref_format struct instead of simple char*.
> Need that for further reusing of formatting logic from ref-filter.

OK, this makes sense (though again, at some point we want this to stop
being a "ref_format" and just be a "format").

>  struct batch_options {
> + struct ref_format *format;

Does this need to be a pointer? We can just store the ref_format inside
the struct, right? And then...

> @@ -557,7 +558,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
> *prefix)
>  {
>   int opt = 0;
>   const char *exp_type = NULL, *obj_name = NULL;
> - struct batch_options batch = {0};
> + struct ref_format format = REF_FORMAT_INIT;
> + struct batch_options batch = {};
>   int unknown_type = 0;

...here you would not need the extra local variable. You can initialize
it like:

  struct batch_options batch = { REF_FORMAT_INIT };

-Peff

[PATCH v2 02/18] cat-file: reuse struct ref_format

[Olga Telezhnaya]

Start using ref_format struct instead of simple char*.
Need that for further reusing of formatting logic from ref-filter.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 16 +---
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index f783b39b9bd5c..7655a9a726773 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -13,15 +13,16 @@
 #include "tree-walk.h"
 #include "sha1-array.h"
 #include "packfile.h"
+#include "ref-filter.h"
 
 struct batch_options {
+   struct ref_format *format;
int enabled;
int follow_symlinks;
int print_contents;
int buffer_output;
int all_objects;
int cmdmode; /* may be 'w' or 'c' for --filters or --textconv */
-   const char *format;
 };
 
 static const char *force_path;
@@ -353,7 +354,7 @@ static void batch_object_write(const char *obj_name, struct 
batch_options *opt,
return;
}
 
-   strbuf_expand(, opt->format, expand_format, data);
+   strbuf_expand(, opt->format->format, expand_format, data);
strbuf_addch(, '\n');
batch_write(opt, buf.buf, buf.len);
strbuf_release();
@@ -446,8 +447,8 @@ static int batch_objects(struct batch_options *opt)
int save_warning;
int retval = 0;
 
-   if (!opt->format)
-   opt->format = "%(objectname) %(objecttype) %(objectsize)";
+   if (!opt->format->format)
+   opt->format->format = "%(objectname) %(objecttype) 
%(objectsize)";
 
/*
 * Expand once with our special mark_query flag, which will prime the
@@ -456,7 +457,7 @@ static int batch_objects(struct batch_options *opt)
 */
memset(, 0, sizeof(data));
data.mark_query = 1;
-   strbuf_expand(, opt->format, expand_format, );
+   strbuf_expand(, opt->format->format, expand_format, );
data.mark_query = 0;
if (opt->cmdmode)
data.split_on_whitespace = 1;
@@ -548,7 +549,7 @@ static int batch_option_callback(const struct option *opt,
 
bo->enabled = 1;
bo->print_contents = !strcmp(opt->long_name, "batch");
-   bo->format = arg;
+   bo->format->format = arg;
 
return 0;
 }
@@ -557,7 +558,8 @@ int cmd_cat_file(int argc, const char **argv, const char 
*prefix)
 {
int opt = 0;
const char *exp_type = NULL, *obj_name = NULL;
-   struct batch_options batch = {0};
+   struct ref_format format = REF_FORMAT_INIT;
+   struct batch_options batch = {};
int unknown_type = 0;
 
const struct option options[] = {

--
https://github.com/git/git/pull/450