Re: [PATCH] userdiff: improve java regex for generic return types

From: Jeff King <peff@peff.net>
Date: Aug 9, 2021, 21:00 UTC
Message-ID: <YRGXhqW0BmKQdeZu@coredump.intra.peff.net>
In-Reply-To: <20210809203308.30128-1-tsdh@gnu.org>

On Mon, Aug 09, 2021 at 10:33:08PM +0200, Tassilo Horn wrote:

> Currently, the git diff hunk headers show the wrong method signature if the
> method has a generic return type because the regex doesn't allow < and > in the
> return type.  This patch adds those.

Thanks. It has been long enough since I used Java that there were no generics back then, but I will take your word that this is what they look like. ;)

The patch itself looks OK to me, but...

Show 17 quoted lines

> when adding the number 6 to the Arrays.asList() call in secondMethod(), the git
> diff one gets is
> 
> --8<---------------cut here---------------start------------->8---
> diff --git a/src/main/java/MyExample.java b/src/main/java/MyExample.java
> index a0f1a6b..ea37a98 100644
> --- a/src/main/java/MyExample.java
> +++ b/src/main/java/MyExample.java
> @@ -14,6 +14,6 @@ public void firstMethod() {
>          // followed by this,
>          // and that,
>          // and even more.
> -        return Arrays.asList(1, 2, 3, 4, 5);
> +        return Arrays.asList(1, 2, 3, 4, 5, 6);
>      }
>  }
> --8<---------------cut here---------------end--------------->8---

...this diff in the commit message will screw up git-am. The usual procedure is to omit the scissors lines and just indent it.

But...

>  userdiff.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

...even better than explaining it is adding a regression test, perhaps like this one:

diff --git a/t/t4018/java-return-generic b/t/t4018/java-return-generic
new file mode 100644
index 0000000000..d030c26184
--- /dev/null
+++ b/t/t4018/java-return-generic
@@ -0,0 +1,10 @@
+class MyExample {
+    public void firstMethod() {
+        // Whatever...
+    }
+
+    public List<String> secondMethod(String RIGHT[]) {
+        // Whatever...
+        return Arrays.asList("ChangeMe");
+    }
+}

(the "ChangeMe" line will be changed, and we'll expect that the line
with "RIGHT" on it is found. We use -U1 to reduce the need for filler).

-Peff