On Sat, Nov 19, 2016 at 1:28 AM, Jakub Narębski <jnareb@xxxxxxxxx> wrote:
> W dniu 08.11.2016 o 21:11, Karthik Nayak pisze:
>> From: Karthik Nayak <karthik.188@xxxxxxxxx>
>>
>> Implement %(if:equals=<string>) wherein the if condition is only
>> satisfied if the value obtained between the %(if:...) and %(then) atom
>> is the same as the given '<string>'.
>>
>> Similarly, implement (if:notequals=<string>) wherein the if condition
>> is only satisfied if the value obtained between the %(if:...) and
>> %(then) atom is differnt from the given '<string>'.
> ^^^^^^^^
>
> s/differnt/different/ <-- typo
>
Will change that.
>>
>> Add tests and Documentation for the same.
>>
>> Mentored-by: Christian Couder <christian.couder@xxxxxxxxx>
>> Mentored-by: Matthieu Moy <matthieu.moy@xxxxxxxxxxxxxxx>
>> Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx>
>> ---
>> Documentation/git-for-each-ref.txt | 3 +++
>> ref-filter.c | 43 +++++++++++++++++++++++++++++++++-----
>> t/t6302-for-each-ref-filter.sh | 18 ++++++++++++++++
>> 3 files changed, 59 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>> index fed8126..b7b8560 100644
>> --- a/Documentation/git-for-each-ref.txt
>> +++ b/Documentation/git-for-each-ref.txt
>> @@ -155,6 +155,9 @@ if::
>> evaluating the string before %(then), this is useful when we
>> use the %(HEAD) atom which prints either "*" or " " and we
>> want to apply the 'if' condition only on the 'HEAD' ref.
>
> So %(if) is actually %(if:notempty) ? Just kidding.
>
It's not a bug, it's a feature ;)
>> + Append ":equals=<string>" or ":notequals=<string>" to compare
>> + the value between the %(if:...) and %(then) atoms with the
>> + given string.
>>
>> In addition to the above, for commit and tag objects, the header
>> field names (`tree`, `parent`, `object`, `type`, and `tag`) can
>> diff --git a/ref-filter.c b/ref-filter.c
>> index 8392303..44481c3 100644
>> --- a/ref-filter.c
>> +++ b/ref-filter.c
>> @@ -22,6 +22,8 @@ struct align {
>> };
>>
>> struct if_then_else {
>> + const char *if_equals,
>> + *not_equals;
>
> I guess using anonymous structs from C11 here...
>
>> unsigned int then_atom_seen : 1,
>> else_atom_seen : 1,
>> condition_satisfied : 1;
>> @@ -49,6 +51,10 @@ static struct used_atom {
>> enum { C_BARE, C_BODY, C_BODY_DEP, C_LINES, C_SIG, C_SUB } option;
>> unsigned int nlines;
>> } contents;
>> + struct {
>> + const char *if_equals,
>> + *not_equals;
>> + } if_then_else;
>
> ...to avoid code duplication there is rather out of question?
>
I believe it holds better context without the use of anonymous structs/unions.
But that's my perspective, I wouldn't mind changing it.
>> enum { O_FULL, O_SHORT } objectname;
>> } u;
>> } *used_atom;
>> @@ -169,6 +175,19 @@ static void align_atom_parser(struct used_atom *atom, const char *arg)
>> string_list_clear(¶ms, 0);
>> }
>>
>> +static void if_atom_parser(struct used_atom *atom, const char *arg)
>> +{
>> + if (!arg)
>> + return;
>> + else if (skip_prefix(arg, "equals=", &atom->u.if_then_else.if_equals))
>> + ;
>> + else if (skip_prefix(arg, "notequals=", &atom->u.if_then_else.not_equals))
>> + ;
>
> Those ';' should be perfectly aligned, isn't it?
>
This should be dropped with the new changes made on this patch.
--
Regards,
Karthik Nayak