diff mbox series

[v13,1/3] grep: refactor next_match() and match_one_pattern() for external use

Message ID 20211015161356.3372-1-someguy@effective-light.com (mailing list archive)
State Accepted
Commit 3f566c4e695a6df8237c34b7c1f34f0832b7e575
Headers show
Series [v13,1/3] grep: refactor next_match() and match_one_pattern() for external use | expand

Commit Message

Hamza Mahfooz Oct. 15, 2021, 4:13 p.m. UTC 
These changes are made in preparation of, the colorization support for the
"git log" subcommands that, rely on regex functionality (i.e. "--author",
"--committer" and "--grep"). These changes are necessary primarily because
match_one_pattern() expects header lines to be prefixed, however, in
pretty, the prefixes are stripped from the lines because the name-email
pairs need to go through additional parsing, before they can be printed and
because next_match() doesn't handle the case of
"ctx == GREP_CONTEXT_HEAD" at all. So, teach next_match() how to handle the
new case and move match_one_pattern()'s core logic to
headerless_match_one_pattern() while preserving match_one_pattern()'s uses
that depend on the additional processing.

Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
---
v5: separate grep changes from pretty changes.

v6: rescope some variables.

v7: export header_field[] and allow for subsequent matches on header lines
    in match_one_pattern().

v8: add headerless_match_one_pattern() and move header_field[] back.

v9: get rid of the new check in headerless_match_one_pattern(), move the
    header pattern filtering logic in grep_next_match() and document
    grep_next_match() in grep.h.

v10: add a "magic" comment in grep_next_match() to signify a fall through
     in the switch statement.
---
 grep.c | 79 ++++++++++++++++++++++++++++++++++++----------------------
 grep.h |  9 +++++++
 2 files changed, 58 insertions(+), 30 deletions(-)

Comments

Junio C Hamano Oct. 15, 2021, 6:05 p.m. UTC | #1 
Hamza Mahfooz <someguy@effective-light.com> writes:

> These changes are made in preparation of, the colorization support for the
> "git log" subcommands that, rely on regex functionality (i.e. "--author",
> "--committer" and "--grep"). These changes are necessary primarily because
> match_one_pattern() expects header lines to be prefixed, however, in
> pretty, the prefixes are stripped from the lines because the name-email
> pairs need to go through additional parsing, before they can be printed and
> because next_match() doesn't handle the case of
> "ctx == GREP_CONTEXT_HEAD" at all. So, teach next_match() how to handle the
> new case and move match_one_pattern()'s core logic to
> headerless_match_one_pattern() while preserving match_one_pattern()'s uses
> that depend on the additional processing.
>
> Signed-off-by: Hamza Mahfooz <someguy@effective-light.com>
> ---
> v5: separate grep changes from pretty changes.
>
> v6: rescope some variables.
>
> v7: export header_field[] and allow for subsequent matches on header lines
>     in match_one_pattern().
>
> v8: add headerless_match_one_pattern() and move header_field[] back.
>
> v9: get rid of the new check in headerless_match_one_pattern(), move the
>     header pattern filtering logic in grep_next_match() and document
>     grep_next_match() in grep.h.
>
> v10: add a "magic" comment in grep_next_match() to signify a fall through
>      in the switch statement.

Makes readers curious what happend in v11 and later...
Hamza Mahfooz Oct. 15, 2021, 6:24 p.m. UTC | #2 
On Fri, Oct 15 2021 at 11:05:24 AM -0700, Junio C Hamano 
<gitster@pobox.com> wrote:
> Makes readers curious what happend in v11 and later...

It was my understanding that no comment means nothing changed since 
then, or is something to the effect of "no changes" preferable?
Junio C Hamano Oct. 15, 2021, 7:28 p.m. UTC | #3 
Hamza Mahfooz <someguy@effective-light.com> writes:

> On Fri, Oct 15 2021 at 11:05:24 AM -0700, Junio C Hamano
> <gitster@pobox.com> wrote:
>> Makes readers curious what happend in v11 and later...
>
> It was my understanding that no comment means nothing changed since
> then, or is something to the effect of "no changes" preferable?

Usually people do not want to spam the list with a multi-patch
series when they have nothing new to show, unless there are other
reasons to do so, like "there is no change, but I am sending it
again because nobody seemed to have time reviewing the series the
last time", in which case that would make a good explanation to put
there.
Hamza Mahfooz Oct. 15, 2021, 7:40 p.m. UTC | #4 
On Fri, Oct 15 2021 at 12:28:42 PM -0700, Junio C Hamano 
<gitster@pobox.com> wrote:
> 
> Usually people do not want to spam the list with a multi-patch
> series when they have nothing new to show, unless there are other
> reasons to do so, like "there is no change, but I am sending it
> again because nobody seemed to have time reviewing the series the
> last time", in which case that would make a good explanation to put
> there.

I made changes to other patches in the series however, (I only document 
the changes if that patch in particular has changed.)
Junio C Hamano Oct. 15, 2021, 7:49 p.m. UTC | #5 
Junio C Hamano <gitster@pobox.com> writes:

> Hamza Mahfooz <someguy@effective-light.com> writes:
>
>> On Fri, Oct 15 2021 at 11:05:24 AM -0700, Junio C Hamano
>> <gitster@pobox.com> wrote:
>>> Makes readers curious what happend in v11 and later...
>>
>> It was my understanding that no comment means nothing changed since
>> then, or is something to the effect of "no changes" preferable?
>
> Usually people do not want to spam the list with a multi-patch
> series when they have nothing new to show, unless there are other
> reasons to do so, like "there is no change, but I am sending it
> again because nobody seemed to have time reviewing the series the
> last time", in which case that would make a good explanation to put
> there.

Ah, I thought I was responding to a cover letter.  OK, so you were
saying that there was no change in 1/3, but other steps may be
different.

In that case, that makes sense (even though it is more helpful to
just say "v13: same as v12" or something).

Thanks.
diff mbox series

Patch

diff --git a/grep.c b/grep.c
index 14fe8a0fd2..fe847a0111 100644
--- a/grep.c
+++ b/grep.c
@@ -944,10 +944,10 @@  static struct {
 	{ "reflog ", 7 },
 };
 
-static int match_one_pattern(struct grep_pat *p,
-			     const char *bol, const char *eol,
-			     enum grep_context ctx,
-			     regmatch_t *pmatch, int eflags)
+static int headerless_match_one_pattern(struct grep_pat *p,
+					const char *bol, const char *eol,
+					enum grep_context ctx,
+					regmatch_t *pmatch, int eflags)
 {
 	int hit = 0;
 	const char *start = bol;
@@ -956,25 +956,6 @@  static int match_one_pattern(struct grep_pat *p,
 	    ((p->token == GREP_PATTERN_HEAD) != (ctx == GREP_CONTEXT_HEAD)))
 		return 0;
 
-	if (p->token == GREP_PATTERN_HEAD) {
-		const char *field;
-		size_t len;
-		assert(p->field < ARRAY_SIZE(header_field));
-		field = header_field[p->field].field;
-		len = header_field[p->field].len;
-		if (strncmp(bol, field, len))
-			return 0;
-		bol += len;
-		switch (p->field) {
-		case GREP_HEADER_AUTHOR:
-		case GREP_HEADER_COMMITTER:
-			strip_timestamp(bol, &eol);
-			break;
-		default:
-			break;
-		}
-	}
-
  again:
 	hit = patmatch(p, bol, eol, pmatch, eflags);
 
@@ -1025,6 +1006,36 @@  static int match_one_pattern(struct grep_pat *p,
 	return hit;
 }
 
+static int match_one_pattern(struct grep_pat *p,
+			     const char *bol, const char *eol,
+			     enum grep_context ctx, regmatch_t *pmatch,
+			     int eflags)
+{
+	const char *field;
+	size_t len;
+
+	if (p->token == GREP_PATTERN_HEAD) {
+		assert(p->field < ARRAY_SIZE(header_field));
+		field = header_field[p->field].field;
+		len = header_field[p->field].len;
+		if (strncmp(bol, field, len))
+			return 0;
+		bol += len;
+
+		switch (p->field) {
+		case GREP_HEADER_AUTHOR:
+		case GREP_HEADER_COMMITTER:
+			strip_timestamp(bol, &eol);
+			break;
+		default:
+			break;
+		}
+	}
+
+	return headerless_match_one_pattern(p, bol, eol, ctx, pmatch, eflags);
+}
+
+
 static int match_expr_eval(struct grep_opt *opt, struct grep_expr *x,
 			   const char *bol, const char *eol,
 			   enum grep_context ctx, ssize_t *col,
@@ -1143,7 +1154,7 @@  static int match_next_pattern(struct grep_pat *p,
 {
 	regmatch_t match;
 
-	if (!match_one_pattern(p, bol, eol, ctx, &match, eflags))
+	if (!headerless_match_one_pattern(p, bol, eol, ctx, &match, eflags))
 		return 0;
 	if (match.rm_so < 0 || match.rm_eo < 0)
 		return 0;
@@ -1158,19 +1169,26 @@  static int match_next_pattern(struct grep_pat *p,
 	return 1;
 }
 
-static int next_match(struct grep_opt *opt,
-		      const char *bol, const char *eol,
-		      enum grep_context ctx, regmatch_t *pmatch, int eflags)
+int grep_next_match(struct grep_opt *opt,
+		    const char *bol, const char *eol,
+		    enum grep_context ctx, regmatch_t *pmatch,
+		    enum grep_header_field field, int eflags)
 {
 	struct grep_pat *p;
 	int hit = 0;
 
 	pmatch->rm_so = pmatch->rm_eo = -1;
 	if (bol < eol) {
-		for (p = opt->pattern_list; p; p = p->next) {
+		for (p = ((ctx == GREP_CONTEXT_HEAD)
+			   ? opt->header_list : opt->pattern_list);
+			  p; p = p->next) {
 			switch (p->token) {
-			case GREP_PATTERN: /* atom */
 			case GREP_PATTERN_HEAD:
+				if ((field != GREP_HEADER_FIELD_MAX) &&
+				    (p->field != field))
+					continue;
+				/* fall thru */
+			case GREP_PATTERN: /* atom */
 			case GREP_PATTERN_BODY:
 				hit |= match_next_pattern(p, bol, eol, ctx,
 							  pmatch, eflags);
@@ -1261,7 +1279,8 @@  static void show_line(struct grep_opt *opt,
 			else if (sign == '=')
 				line_color = opt->colors[GREP_COLOR_FUNCTION];
 		}
-		while (next_match(opt, bol, eol, ctx, &match, eflags)) {
+		while (grep_next_match(opt, bol, eol, ctx, &match,
+				       GREP_HEADER_FIELD_MAX, eflags)) {
 			if (match.rm_so == match.rm_eo)
 				break;
 
diff --git a/grep.h b/grep.h
index 3cb8a83ae8..808ad76f0c 100644
--- a/grep.h
+++ b/grep.h
@@ -191,6 +191,15 @@  void compile_grep_patterns(struct grep_opt *opt);
 void free_grep_patterns(struct grep_opt *opt);
 int grep_buffer(struct grep_opt *opt, const char *buf, unsigned long size);
 
+/* The field parameter is only used to filter header patterns
+ * (where appropriate). If filtering isn't desirable
+ * GREP_HEADER_FIELD_MAX should be supplied.
+ */
+int grep_next_match(struct grep_opt *opt,
+		    const char *bol, const char *eol,
+		    enum grep_context ctx, regmatch_t *pmatch,
+		    enum grep_header_field field, int eflags);
+
 struct grep_source {
 	char *name;