Re: [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter

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

2018-02-15 8:51 GMT+03:00 Jeff King :
> On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:
>
>> Move logic related to skip_object_info into ref-filter,
>> so that cat-file does not use that field at all.
>
> I think this is going the wrong way. ref-filter should always do as
> little work as possible to fulfill the request. So it should skip the
> object_info call whenever it can. And then any callers who want to make
> sure that the object exists can do so (as long as the formatting code
> tells them whether it looked up the object or not).
>
> And then ref-filter doesn't have to know about this skip_object_info
> flag at all.

Your message looks contradictory to me.
I agree that ref-filter should do as least as it's possible, and that
is the main reason why I put this code there. Moreover, I think that
it's a good idea to implement that variable not only for cat-file, but
for all ref-filter callers. And I think that it's a task of ref-filter
to check whether the object exists or not (or - whether the ref is
valid or not). But I am not sure that I need to solve that moment in
current patch. It sounds like another separate task.

>
> -Peff

Re: [PATCH v3 15/23] cat-file: move skip_object_info into ref-filter

[Jeff King]

On Mon, Feb 12, 2018 at 08:08:54AM +, Olga Telezhnaya wrote:

> Move logic related to skip_object_info into ref-filter,
> so that cat-file does not use that field at all.

I think this is going the wrong way. ref-filter should always do as
little work as possible to fulfill the request. So it should skip the
object_info call whenever it can. And then any callers who want to make
sure that the object exists can do so (as long as the formatting code
tells them whether it looked up the object or not).

And then ref-filter doesn't have to know about this skip_object_info
flag at all.

-Peff

[PATCH v3 15/23] cat-file: move skip_object_info into ref-filter

[Olga Telezhnaya]

Move logic related to skip_object_info into ref-filter,
so that cat-file does not use that field at all.

Signed-off-by: Olga Telezhnaia 
Mentored-by: Christian Couder 
Mentored by: Jeff King 
---
 builtin/cat-file.c | 7 +--
 ref-filter.c   | 5 +
 ref-filter.h   | 1 +
 3 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 3a49b55a1cc2e..582679f3dca2c 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -393,14 +393,9 @@ static int batch_objects(struct batch_options *opt)
 */
memset(, 0, sizeof(data));
opt->format.cat_file_data = 
+   opt->format.all_objects = opt->all_objects;
verify_ref_format(>format);
 
-   if (opt->all_objects) {
-   struct object_info empty = OBJECT_INFO_INIT;
-   if (!memcmp(, , sizeof(empty)))
-   data.skip_object_info = 1;
-   }
-
/*
 * If we are printing out the object, then always fill in the type,
 * since we will want to decide whether or not to stream.
diff --git a/ref-filter.c b/ref-filter.c
index 4adeea6aad0da..104cd6aef0102 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -775,6 +775,11 @@ int verify_ref_format(struct ref_format *format)
}
if (format->need_color_reset_at_eol && !want_color(format->use_color))
format->need_color_reset_at_eol = 0;
+   if (cat_file_info && format->all_objects) {
+   struct object_info empty = OBJECT_INFO_INIT;
+   if (!memcmp(_file_info->info, , sizeof(empty)))
+   cat_file_info->skip_object_info = 1;
+   }
return 0;
 }
 
diff --git a/ref-filter.h b/ref-filter.h
index fffc443726e28..b1c668c12428b 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -118,6 +118,7 @@ struct ref_format {
 * hopefully would be reduced later.
 */
struct expand_data *cat_file_data;
+   unsigned all_objects : 1;
 };
 
 #define REF_FORMAT_INIT { NULL, 0, -1 }

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