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

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

[Kapil Jain]

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.

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

[Olga Telezhnaya]

пн, 25 мар. 2019 г. в 22:27, Kapil Jain <jkapil.cs@gmail.com>:
>
> 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.

> 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.

>
> 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

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

[Kapil Jain]

> 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 ?

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.

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

[Olga Telezhnaya]

ср, 27 мар. 2019 г. в 20:01, Kapil Jain <jkapil.cs@gmail.com>:
>
> > 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.

>
> 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.

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

[Kapil Jain]

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.

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

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

[Kapil Jain]

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 ?

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

[Thomas Gummerer]

On 03/31, Kapil Jain wrote:
> 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.

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

[Olga Telezhnaya]

вс, 31 мар. 2019 г. в 20:45, Kapil Jain <jkapil.cs@gmail.com>:
>
> 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

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

[Kapil Jain]

On Mon, Apr 1, 2019 at 12:19 AM Thomas Gummerer <t.gummerer@gmail.com> wrote:
>
> 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.