Skip to content

FR: git diff output compatibility, function label for hunks #4797

@botovq

Description

@botovq

git's diff has a heuristic to determine what function a patch hunk applies to: it prints what it determines to be the function name after the second @@. Some diff implementations have variants of this under the -p option (or --show-c-function in GNU diff). This is tremendously helpful for reviewing patches in text files. I noticed I use this all the time when composing commit messages and it's seemingly missing from jj:

https://github.com/martinvonz/jj/blob/e55d03a2ee611d08f09a38e01e0cb47c1d6cd315/cli/src/diff_util.rs#L1406-L1413

Compare git's output containing @@ -123,7 +123,7 @@ tls_get_peer_cert_common_name(struct tls *ctx, char **common_name)

$ git show 67f33188 | cat
commit 67f33188bdca6d7612732a59795cf65c184f00ab
Author: Theo Buehler <[email protected]>
Date:   Thu Nov 7 15:00:24 2024 +0100

diff --git a/lib/libtls/tls_conninfo.c b/lib/libtls/tls_conninfo.c
index 30a4e9403025..1cffba0c3da6 100644
--- a/lib/libtls/tls_conninfo.c
+++ b/lib/libtls/tls_conninfo.c
@@ -123,7 +123,7 @@ tls_get_peer_cert_common_name(struct tls *ctx, char **common_name)
 {
 	if (ctx->ssl_peer_cert == NULL)
 		return (-1);
-	return tls_get_common_name(ctx, ctx->ssl_peer_cert, common_name);
+	return tls_get_common_name(ctx, ctx->ssl_peer_cert, NULL, common_name);
 }

 static int

with jujutsu's output where the function name is missing and only @@ -123,7 +123,7 @@ is shown:

$ jj show --git --no-pager
Commit ID: 67f33188bdca6d7612732a59795cf65c184f00ab
Change ID: klzopryvxnuswttvuynokmlxkrzpsrxy
Author: Theo Buehler <[email protected]> (2024-11-07 15:00:24)
Committer: Theo Buehler <[email protected]> (2024-11-08 01:38:34)

    (no description set)

diff --git a/lib/libtls/tls_conninfo.c b/lib/libtls/tls_conninfo.c
index 30a4e94030..1cffba0c3d 100644
--- a/lib/libtls/tls_conninfo.c
+++ b/lib/libtls/tls_conninfo.c
@@ -123,7 +123,7 @@
 {
 	if (ctx->ssl_peer_cert == NULL)
 		return (-1);
-	return tls_get_common_name(ctx, ctx->ssl_peer_cert, common_name);
+	return tls_get_common_name(ctx, ctx->ssl_peer_cert, NULL, common_name);
 }

 static int

The OpenBSD diff manual gives a hint how this could be implemented. I haven't looked at GNU diff and git-diff code - I expect their heuristics to be very similar:

With unified and context diffs, show with each change the first 40 characters of the last line before the context beginning with a letter, an underscore or a dollar sign. For C source code following standard layout conventions, this will show the prototype of the function the change applies to.

Of course this isn't perfect. One well-known problem with this heuristic is that C goto labels tend to take up the spot of the function name, which in turn leads to a common workaround/style convention to add a space in front of the label to get the function name again.

While I'm not sure this is helpful in all languages, and may be considered visual clutter by some, I find this invaluable and a way of having this available would be super helpful.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions