threads / discuss / 50819

[GSoC] Unify ref-filter formats with other --pretty formats

Subject: [GSoC] Unify ref-filter formats with other --pretty formats

## tl;dr

9 messages between Mar 25, 2019 and Apr 1, 2019.

replies: 8people: 3as markdown or json

Kapil Jain· Mar 25, 2019, 19:26 UTC · lore
Hi,

Below are some two queries concerning https://git.github.io/SoC-2019-Ideas/#unify-ref-filter-formats-with-other---pretty-formats

Q1)

In pretty.h & pretty.c: void get_commit_format(const char *arg, struct rev_info *); This function Parses given arguments from "arg", checks it for correctness and * fill struct rev_info.

In ref-filter.h & ref-filter.c: int verify_ref_format(struct ref_format *format); This function is Used to verify if the given format is correct and to parse out the used atoms.

Now, the verify_ref_format function can be used inside get_commit_format function, hence reusing logic. Is this a correct example to work on, for this project ? If not, please point out an example so as to understand the problem statement better.

Other than this I can't find any other example, for this project in pretty.* and ref-filter.* Perhaps some examples could be found in command specific files, right ?

Q2) About a recurring term 'atom' in ref-filter and pretty: what is atom ? is it a piece of a whole document ? and what is meant by used atoms ?

Thanks.
Olga Telezhnaya· Mar 25, 2019, 21:17 UTC · re: Kapil Jain · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

пн, 25 мар. 2019 г. в 22:27, Kapil Jain <jkapil.cs@gmail.com>:
Show 21 quoted lines
>
> Hi,
>
> Below are some two queries concerning
> https://git.github.io/SoC-2019-Ideas/#unify-ref-filter-formats-with-other---pretty-formats
>
> Q1)
>
> In pretty.h & pretty.c:
> void get_commit_format(const char *arg, struct rev_info *);
> This function Parses given arguments from "arg", checks it for
> correctness and * fill struct rev_info.
>
> In ref-filter.h & ref-filter.c:
> int verify_ref_format(struct ref_format *format);
> This function is Used to verify if the given format is correct and to
> parse out the used atoms.
>
> Now, the verify_ref_format function can be used inside
> get_commit_format function, hence reusing logic.
> Is this a correct example to work on, for this project ?
Hi! Yes, in my opinion your example looks like good starting point.
Show 6 quoted lines
> If not, please point out an example so as to understand the problem
> statement better.
>
> Other than this I can't find any other example, for this project in
> pretty.* and ref-filter.*
> Perhaps some examples could be found in command specific files, right ?

Other parts of the project are about reusing other ref-filter logic. For example, we could try to reuse format_ref_array_item() from ref-filter.h. I haven't dig into pretty.c logic much, but I guess it is possible to translate "pretty" formatting commands to ref-filter ones. That will allow us to remove similar logic from pretty.c. Our final goal is to minimise code duplication and to have one unified interface to extract all needed data from object and to print it properly.

Show 5 quoted lines
>
> Q2)
> About a recurring term 'atom' in ref-filter and pretty:
> what is atom ? is it a piece of a whole document ? and what is meant
> by used atoms ?

I had the same question in my beginning. Please have a look at [1]. Another good question - what is object. You could ensure that you understand this by reading [2].

>
> Thanks.

[1] https://git-scm.com/docs/git-for-each-ref#_field_names [2] https://git-scm.com/book/en/v2/Git-Internals-Git-Objects

Kapil Jain· Mar 27, 2019, 17:01 UTC · re: Olga Telezhnaya · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

Show 7 quoted lines
> On Tue, Mar 26, 2019 at 2:48 AM Olga Telezhnaya <olyatelezhnaya@gmail.com> wrote:
>> Kapil Jain <jkapil.cs@gmail.com> wrote:
> > Now, the verify_ref_format function can be used inside
> > get_commit_format function, hence reusing logic.
> > Is this a correct example to work on, for this project ?
>
> Hi! Yes, in my opinion your example looks like good starting point.

I read through the code of both functions, and I think they are different. Please point out if I missed to see the similarity.

or may be it seemed that way, because they both deal with different formats. So, first should a translating function (pretty to ref-filter) be written ?

Show 5 quoted lines
> > Other than this I can't find any other example, for this project in
> > pretty.* and ref-filter.*
> > Perhaps some examples could be found in command specific files, right ?
>
> Other parts of the project are about reusing other ref-filter logic.

So, the project is not limited to reusing ref-filter logics in pretty, it is about reusing ref-filter logic wherever possible, right ?

> For example, we could try to reuse format_ref_array_item() from
> ref-filter.h.
where can format_ref_array_item() be reused ?
Show 6 quoted lines
> I haven't dig into pretty.c logic much, but I guess it
> is possible to translate "pretty" formatting commands to ref-filter
> ones. That will allow us to remove similar logic from pretty.c. Our
> final goal is to minimise code duplication and to have one unified
> interface to extract all needed data from object and to print it
> properly.

I looked, and yes some, but not all pretty formats are translatable. For example: %GP, %p, %P. are not translatable to ref-filter. or is there a workaround to translate them ?

It looks like to reuse ref-filter logic, a translator from pretty to ref-filter needs to be built. So, building a translator would be a starting point ? and then second step would be to recognise places where ref-filter can be reused, right ?

Show 9 quoted lines
> > what is atom ? is it a piece of a whole document ? and what is meant
> > by used atoms ?
>
> I had the same question in my beginning. Please have a look at [1].
> Another good question - what is object. You could ensure that you
> understand this by reading [2].
>
> [1] https://git-scm.com/docs/git-for-each-ref#_field_names
> [2] https://git-scm.com/book/en/v2/Git-Internals-Git-Objects
Thanks, this helped.
Olga Telezhnaya· Mar 28, 2019, 17:43 UTC · re: Kapil Jain · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

ср, 27 мар. 2019 г. в 20:01, Kapil Jain <jkapil.cs@gmail.com>:
Show 40 quoted lines
>
> > On Tue, Mar 26, 2019 at 2:48 AM Olga Telezhnaya <olyatelezhnaya@gmail.com> wrote:
> >> Kapil Jain <jkapil.cs@gmail.com> wrote:
> > > Now, the verify_ref_format function can be used inside
> > > get_commit_format function, hence reusing logic.
> > > Is this a correct example to work on, for this project ?
> >
> > Hi! Yes, in my opinion your example looks like good starting point.
>
> I read through the code of both functions, and I think they are different.
> Please point out if I missed to see the similarity.
>
> or may be it seemed that way, because they both deal with different formats.
> So, first should a translating function (pretty to ref-filter) be written ?
>
>
> > > Other than this I can't find any other example, for this project in
> > > pretty.* and ref-filter.*
> > > Perhaps some examples could be found in command specific files, right ?
> >
> > Other parts of the project are about reusing other ref-filter logic.
>
> So, the project is not limited to reusing ref-filter logics in pretty,
> it is about reusing ref-filter logic wherever possible, right ?
>
> > For example, we could try to reuse format_ref_array_item() from
> > ref-filter.h.
>
> where can format_ref_array_item() be reused ?
>
> > I haven't dig into pretty.c logic much, but I guess it
> > is possible to translate "pretty" formatting commands to ref-filter
> > ones. That will allow us to remove similar logic from pretty.c. Our
> > final goal is to minimise code duplication and to have one unified
> > interface to extract all needed data from object and to print it
> > properly.
>
> I looked, and yes some, but not all pretty formats are translatable.
> For example: %GP, %p, %P. are not translatable to ref-filter. or is
> there a workaround to translate them ?

I will try to answer to all your questions here in general way. Thomas, Christian, please correct me if you disagree. Our main goal is to simplify codebase, get rid of duplication of similar logic and, as a result, simplify adding new functionality. We also need to save backward compatibility, so we can't just delete some commands, rewrite them and change their interface (unfortunately). You are free to suggest your own ideas how to achieve these goals, and you are free to choose exact tasks. Yes, you are not limited only to pretty.c, but it is a good place where to start.

I was an intern in winter 2017-2018 and I was trying to get rid of formatting logic in cat-file. You may try to do same thing in pretty.c. I guess it's easier to think how to reuse ref-filter in pretty.c because ref-filter has the most general interface between all these files. But, if you are sure that you have better idea - I am strongly recommend you to share it with the mailing list.

Unfortunately, I can't consult you properly about structure of pretty.c. I guess that would be your first task of the internship to dive into it and think how to improve it. By the way, you could try to make more detailed documentation and that could be one of your first contributions. It will help you to understand the system better, and other contributors will be happy to read it.

Show 18 quoted lines
>
> It looks like to reuse ref-filter logic, a translator from pretty to
> ref-filter needs to be built.
> So, building a translator would be a starting point ?
> and then second step would be to recognise places where ref-filter can
> be reused, right ?
>
> > > what is atom ? is it a piece of a whole document ? and what is meant
> > > by used atoms ?
> >
> > I had the same question in my beginning. Please have a look at [1].
> > Another good question - what is object. You could ensure that you
> > understand this by reading [2].
> >
> > [1] https://git-scm.com/docs/git-for-each-ref#_field_names
> > [2] https://git-scm.com/book/en/v2/Git-Internals-Git-Objects
>
> Thanks, this helped.
Kapil Jain· Mar 29, 2019, 13:53 UTC · re: Olga Telezhnaya · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

On Thu, Mar 28, 2019 at 11:14 PM Olga Telezhnaya <olyatelezhnaya@gmail.com> wrote:

Show 7 quoted lines
>
> Unfortunately, I can't consult you properly about structure of
> pretty.c. I guess that would be your first task of the internship to
> dive into it and think how to improve it. By the way, you could try to
> make more detailed documentation and that could be one of your first
> contributions. It will help you to understand the system better, and
> other contributors will be happy to read it.

ok, i will be reading pretty.c and will document its structure at required places.

Kapil Jain· Mar 31, 2019, 17:45 UTC · re: Kapil Jain · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

On Fri, Mar 29, 2019 at 7:23 PM Kapil Jain <jkapil.cs@gmail.com> wrote:
Show 11 quoted lines
>
> On Thu, Mar 28, 2019 at 11:14 PM Olga Telezhnaya
> <olyatelezhnaya@gmail.com> wrote:
> >
> > Unfortunately, I can't consult you properly about structure of
> > pretty.c. I guess that would be your first task of the internship to
> > dive into it and think how to improve it. By the way, you could try to
> > make more detailed documentation and that could be one of your first
> > contributions. It will help you to understand the system better, and
> > other contributors will be happy to read it.
>

i traced the cmd_log() to understand the point at which pretty.c could be used, i only got to userformat_find_requirements().

struct userformat_want {
    unsigned notes:1;
    unsigned source:1;
};
what are notes and source flags used for ?

olga: what approach did you follow to figure which logic in cat-file was redundant and that ref-filter could be reused there ? does it include picking any file, go through it entirely and then pick places to be replaced by ref-filter logic ?

Thomas Gummerer· Mar 31, 2019, 18:49 UTC · re: Kapil Jain · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

On 03/31, Kapil Jain wrote:
Show 22 quoted lines
> On Fri, Mar 29, 2019 at 7:23 PM Kapil Jain <jkapil.cs@gmail.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 11:14 PM Olga Telezhnaya
> > <olyatelezhnaya@gmail.com> wrote:
> > >
> > > Unfortunately, I can't consult you properly about structure of
> > > pretty.c. I guess that would be your first task of the internship to
> > > dive into it and think how to improve it. By the way, you could try to
> > > make more detailed documentation and that could be one of your first
> > > contributions. It will help you to understand the system better, and
> > > other contributors will be happy to read it.
> >
> 
> i traced the cmd_log() to understand the point at which pretty.c could
> be used, i only got to userformat_find_requirements().
> 
> struct userformat_want {
>     unsigned notes:1;
>     unsigned source:1;
> };
> 
> what are notes and source flags used for ?

If you look at what userformat_find_requirements() does, calls strbuf_expand(), which in turn calls userformat_want_item(), which fills the 'userformat_want' struct based on the strbuf that has been passed.

Now if we look at the caller of userformat_find_requirements(), which is cmd_log_init_finish(), you can see where 'w.notes' and 'w.source' is used.

Just this parsing is probably not something that the ref-filter API can help too much with.

I unfortunately don't have much experience with the pretty, or the ref-filter API. But rather than going into the details of the code already, I'd suggest first looking at what you actually want to replace (see for example the PRETTY FORMATS section in 'man git-log', what which verbs you can use in the ref-filter (see 'man git-for-each-ref') to achieve the same thing.

Then you can see how one format is implemented in 'pretty.c', and see how a similar thing using the ref-filter is implemented in 'ref-filter.c'.

E.g. the "%(objectname:short) %(contents:subject)" (this is missing coloring, but just to get you the idea) is similar to '--pretty=oneline'. Now you can try to find how 'oneline' is implemented in 'pretty.c', and you could translate that to use the 'ref-filter' API.

Kapil Jain· Apr 1, 2019, 12:58 UTC · re: Thomas Gummerer · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

On Mon, Apr 1, 2019 at 12:19 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
Show 10 quoted lines
>
> If you look at what userformat_find_requirements() does, calls
> strbuf_expand(), which in turn calls userformat_want_item(), which
> fills the 'userformat_want' struct based on the strbuf that has been
> passed.
>
> Now if we look at the caller of userformat_find_requirements(), which
> is cmd_log_init_finish(), you can see where 'w.notes' and 'w.source'
> is used.
>

i actually got to userformat_find_requirements() by tracing from cmd_log() itself. although i won't say i read the cmd_log_init_finish() line by line. will do.

Olga Telezhnaya· Mar 31, 2019, 19:28 UTC · re: Kapil Jain · lore

Re: [GSoC] Unify ref-filter formats with other --pretty formats

вс, 31 мар. 2019 г. в 20:45, Kapil Jain <jkapil.cs@gmail.com>:
Show 28 quoted lines
>
> On Fri, Mar 29, 2019 at 7:23 PM Kapil Jain <jkapil.cs@gmail.com> wrote:
> >
> > On Thu, Mar 28, 2019 at 11:14 PM Olga Telezhnaya
> > <olyatelezhnaya@gmail.com> wrote:
> > >
> > > Unfortunately, I can't consult you properly about structure of
> > > pretty.c. I guess that would be your first task of the internship to
> > > dive into it and think how to improve it. By the way, you could try to
> > > make more detailed documentation and that could be one of your first
> > > contributions. It will help you to understand the system better, and
> > > other contributors will be happy to read it.
> >
>
> i traced the cmd_log() to understand the point at which pretty.c could
> be used, i only got to userformat_find_requirements().
>
> struct userformat_want {
>     unsigned notes:1;
>     unsigned source:1;
> };
>
> what are notes and source flags used for ?
>
> olga: what approach did you follow to figure which logic in cat-file
> was redundant and that ref-filter could be reused there ?
> does it include picking any file, go through it entirely and then pick
> places to be replaced by ref-filter logic ?

I just explored how the code works. You could see my commits here [1]. Oh, that's funny, I forgot that I started from creating pretty.h. I could choose between pretty and cat-file, and I made the choice randomly.

In cat-file, interface was so close to ref-filter, but the way of obtaining data was different, and formatting logic was coded twice. We decided that cat-file gets the data more efficiently, and I changed ref-filter logic, it works faster now. Then I needed to reuse formatting logic, and I am still working on that (do not worry, it must not be a reason for merge conflicts).

I guess you will have another workflow: I don't know anything about how pretty gets the data, but the interface differs a lot. So you will have another tasks.

If you have enough skills to debug the code, I definitely suggest you to go through all formatting process step-by-step (both for pretty and ref-filter) for different type of input, that will explain you a lot and maybe that will give you some ideas how to achieve the goals better. 1.5 years ago I didn't know how to use gdb properly, and it was much more important for me to start doing just something, that's why I used debug prints. The meaning is the same anyway.

The most important advice that I can give you: think about whole process, then try to design your steps so that they could be as small as possible. I mean, it's not a good idea to make big patches (more than 3-5 commits), especially at the beginning.

[1] https://github.com/git/git/commits?author=telezhnaya

← back to recent threads