Re: [PATCH v7 03/17] ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)

On Sat, Nov 19, 2016 at 1:28 AM, Jakub Narębski [off-list ref] wrote:

W dniu 08.11.2016 o 21:11, Karthik Nayak pisze:

quoted

From: Karthik Nayak <redacted>

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.

quoted

Add tests and Documentation for the same.

Mentored-by: Christian Couder [off-list ref]
Mentored-by: Matthieu Moy [off-list ref]
Signed-off-by: Karthik Nayak <redacted>
---
 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 ;)

quoted

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

quoted

      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.

quoted

              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(&params, 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