Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

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

2018-01-18 0:45 GMT+03:00 Jeff King :
> On Tue, Jan 16, 2018 at 10:00:42AM +0300, Оля Тележная wrote:
>
>> > I think some of these will want to remain in cat-file.c. For instance,
>> > split_on_whitespace is not something that ref-filter.c itself would care
>> > about. I'd expect in the endgame for cat-file to keep its own
>> > split_on_whitespace flag, and to set it based on the presence of the
>> > %(rest) flag, which it can check by seeing if the "rest" atom is in the
>> > "used_atom" list after parsing the format.
>>
>> Yes, maybe we will need to leave some part of expand_data variables.
>> But, if it would be only "split_on_whitespace", it's better to make
>> just one flag without any other stuff. Actually, I thought about
>> moving related logic to ref-filter also. Anyway, it's hard to say
>> exact things before we code everything. Do I need to fix commit
>> message and make it more soft?
>
> Yeah, I think it might just be split_on_whitespace, and that could live
> on as a single variable in cat-file.c.
>
> I don't think it makes sense to move that part to ref-filter, since it's
> not about formatting at all. It's about how cat-file parses its input,
> which is going to be unlike other ref-filter users (which don't
> generally parse input at all).
>
> -Peff

OK, I will leave split_on_whitespace in cat-file for now. :) There are
so many places that are more important now, in my opinion, so I am
planning just to leave this variable and do other stuff, and I will
return to this question later.

Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

[Jeff King]

On Tue, Jan 16, 2018 at 10:00:42AM +0300, Оля Тележная wrote:

> > I think some of these will want to remain in cat-file.c. For instance,
> > split_on_whitespace is not something that ref-filter.c itself would care
> > about. I'd expect in the endgame for cat-file to keep its own
> > split_on_whitespace flag, and to set it based on the presence of the
> > %(rest) flag, which it can check by seeing if the "rest" atom is in the
> > "used_atom" list after parsing the format.
> 
> Yes, maybe we will need to leave some part of expand_data variables.
> But, if it would be only "split_on_whitespace", it's better to make
> just one flag without any other stuff. Actually, I thought about
> moving related logic to ref-filter also. Anyway, it's hard to say
> exact things before we code everything. Do I need to fix commit
> message and make it more soft?

Yeah, I think it might just be split_on_whitespace, and that could live
on as a single variable in cat-file.c.

I don't think it makes sense to move that part to ref-filter, since it's
not about formatting at all. It's about how cat-file parses its input,
which is going to be unlike other ref-filter users (which don't
generally parse input at all).

-Peff

Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

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

2018-01-16 0:44 GMT+03:00 Jeff King :
> On Wed, Jan 10, 2018 at 09:36:41AM +, Olga Telezhnaya wrote:
>
>> Need that for further reusing of formatting logic in cat-file.
>> Have plans to get rid of using expand_data in cat-file at all,
>> and use it only in ref-filter for collecting, formatting and printing
>> needed data.
>
> I think some of these will want to remain in cat-file.c. For instance,
> split_on_whitespace is not something that ref-filter.c itself would care
> about. I'd expect in the endgame for cat-file to keep its own
> split_on_whitespace flag, and to set it based on the presence of the
> %(rest) flag, which it can check by seeing if the "rest" atom is in the
> "used_atom" list after parsing the format.
>
> -Peff

Yes, maybe we will need to leave some part of expand_data variables.
But, if it would be only "split_on_whitespace", it's better to make
just one flag without any other stuff. Actually, I thought about
moving related logic to ref-filter also. Anyway, it's hard to say
exact things before we code everything. Do I need to fix commit
message and make it more soft?

Olga

Re: [PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

[Jeff King]

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

> Need that for further reusing of formatting logic in cat-file.
> Have plans to get rid of using expand_data in cat-file at all,
> and use it only in ref-filter for collecting, formatting and printing
> needed data.

I think some of these will want to remain in cat-file.c. For instance,
split_on_whitespace is not something that ref-filter.c itself would care
about. I'd expect in the endgame for cat-file to keep its own
split_on_whitespace flag, and to set it based on the presence of the
%(rest) flag, which it can check by seeing if the "rest" atom is in the
"used_atom" list after parsing the format.

-Peff

[PATCH v2 04/18] cat-file: move struct expand_data into ref-filter

[Olga Telezhnaya]

Need that for further reusing of formatting logic in cat-file.
Have plans to get rid of using expand_data in cat-file at all,
and use it only in ref-filter for collecting, formatting and printing
needed data.

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

diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 7655a9a726773..7fd5b960ad698 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -176,42 +176,6 @@ static int cat_one_file(int opt, const char *exp_type, 
const char *obj_name,
return 0;
 }
 
-struct expand_data {
-   struct object_id oid;
-   enum object_type type;
-   unsigned long size;
-   off_t disk_size;
-   const char *rest;
-   struct object_id delta_base_oid;
-
-   /*
-* If mark_query is true, we do not expand anything, but rather
-* just mark the object_info with items we wish to query.
-*/
-   int mark_query;
-
-   /*
-* Whether to split the input on whitespace before feeding it to
-* get_sha1; this is decided during the mark_query phase based on
-* whether we have a %(rest) token in our format.
-*/
-   int split_on_whitespace;
-
-   /*
-* After a mark_query run, this object_info is set up to be
-* passed to sha1_object_info_extended. It will point to the data
-* elements above, so you can retrieve the response from there.
-*/
-   struct object_info info;
-
-   /*
-* This flag will be true if the requested batch format and options
-* don't require us to call sha1_object_info, which can then be
-* optimized out.
-*/
-   unsigned skip_object_info : 1;
-};
-
 static int is_atom(const char *atom, const char *s, int slen)
 {
int alen = strlen(atom);
diff --git a/ref-filter.h b/ref-filter.h
index 0d98342b34319..16d00e4b1bded 100644
--- a/ref-filter.h
+++ b/ref-filter.h
@@ -72,6 +72,42 @@ struct ref_filter {
verbose;
 };
 
+struct expand_data {
+   struct object_id oid;
+   enum object_type type;
+   unsigned long size;
+   off_t disk_size;
+   const char *rest;
+   struct object_id delta_base_oid;
+
+   /*
+* If mark_query is true, we do not expand anything, but rather
+* just mark the object_info with items we wish to query.
+*/
+   int mark_query;
+
+   /*
+* Whether to split the input on whitespace before feeding it to
+* get_sha1; this is decided during the mark_query phase based on
+* whether we have a %(rest) token in our format.
+*/
+   int split_on_whitespace;
+
+   /*
+* After a mark_query run, this object_info is set up to be
+* passed to sha1_object_info_extended. It will point to the data
+* elements above, so you can retrieve the response from there.
+*/
+   struct object_info info;
+
+   /*
+* This flag will be true if the requested batch format and options
+* don't require us to call sha1_object_info, which can then be
+* optimized out.
+*/
+   unsigned skip_object_info : 1;
+};
+
 struct ref_format {
/*
 * Set these to define the format; make sure you call

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