[PATCH v2] grep: improve errors for unmatched ( and )

From: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
Date: Apr 9, 2024, 10:07 UTC
Message-ID: <gkoqujwrzxdt2rxpcbhz5zfspnajdko53wlazaymt5lbce5qch@tarta.nabijaczleweli.xyz>
In-Reply-To: <xmqqzfuox1oi.fsf@gitster.g>

Imagine you want to grep for (. Easy:

  $ git grep '('
  fatal: unmatched parenthesis

uhoh. This is plainly wrong. Unless you know specifically that (a) git grep has expression groups and that (b) the only way to work around them is by doing -e '('

Similarly,

  $ git grep ')'
  fatal: incomplete pattern expression: )

is somehow worse. ")" is a complete regular expression pattern. Of course, the error wants to say "group" here. In this case it's also not "incomplete", it's unmatched. But whatever.

Make them return

  $ ./git grep '('
  fatal: unmatched ( for expression group
  $ ./git grep ')'
  fatal: incomplete pattern expression group: )

which hopefully are clearer in indicating that it's not the expression that's wrong (since no pattern had been parsed at all), but rather that it's been misconstrued as a grouping operator.

Link: https://bugs.debian.org/1051205
Signed-off-by: Ahelenia Ziemiańska <nabijaczleweli@nabijaczleweli.xyz>
---
-- '(' no longer mentioned, otherwise no changes.

 grep.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/grep.c b/grep.c
index 5f23d1a..ac34bfe 100644
--- a/grep.c
+++ b/grep.c
@@ -621,7 +621,7 @@ static struct grep_expr *compile_pattern_atom(struct grep_pat **list)
 		*list = p->next;
 		x = compile_pattern_or(list);
 		if (!*list || (*list)->token != GREP_CLOSE_PAREN)
-			die("unmatched parenthesis");
+			die("unmatched ( for expression group");
 		*list = (*list)->next;
 		return x;
 	default:
@@ -792,7 +792,7 @@ void compile_grep_patterns(struct grep_opt *opt)
 	if (p)
 		opt->pattern_expression = compile_pattern_expr(&p);
 	if (p)
-		die("incomplete pattern expression: %s", p->pattern);
+		die("incomplete pattern expression group: %s", p->pattern);
 
 	if (opt->no_body_match && opt->pattern_expression)
 		opt->pattern_expression = grep_not_expr(opt->pattern_expression);

-- 
2.39.2