How to write comments inside a patch/diff file? [duplicate]

Question

I would like to review a patch of a colleague. We are unable to use a review tool. So I would like to comment the patch file he made. Is it possible to write inline comments to a (svn) patch file?

I couldn't find any information in the svn red book on it. I was even unable to find the patch file grammar to figure it out myself.

Answer 1

The diff format is just the unified diff format. If you wanted you could put some text after the range info. Consider this diff produced with command svn diff -c 1544711 https://svn.apache.org/repos/asf/subversion/trunk:

Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c    (revision 1544710)
+++ subversion/mod_dav_svn/mod_dav_svn.c    (revision 1544711)
@@ -1097,7 +1097,8 @@

 /* Fill the filename on the request with a bogus path since we aren't serving
  * a file off the disk.  This means that <Directory> blocks will not match and
- * that %f in logging formats will show as "svn:/path/to/repo/path/in/repo". */
+ * %f in logging formats will show as "dav_svn:/path/to/repo/path/in/repo".
+ */
 static int dav_svn__translate_name(request_rec *r)
 {
   const char *fs_path, *repos_basename, *repos_path;
@@ -1146,7 +1147,7 @@
   if (repos_path && '/' == repos_path[0] && '\0' == repos_path[1])
     repos_path = NULL;

-  /* Combine 'svn:', fs_path and repos_path to produce the bogus path we're
+  /* Combine 'dav_svn:', fs_path and repos_path to produce the bogus path we're
    * placing in r->filename.  We can't use our standard join helpers such
    * as svn_dirent_join.  fs_path is a dirent and repos_path is a fspath
    * (that can be trivially converted to a relpath by skipping the leading
@@ -1154,7 +1155,7 @@
    * repository is 'trunk/c:hi' this results in a non canonical dirent on
    * Windows. Instead we just cat them together. */
   r->filename = apr_pstrcat(r->pool,
-                            "svn:", fs_path, repos_path, SVN_VA_NULL);
+                            "dav_svn:", fs_path, repos_path, SVN_VA_NULL);

   /* Leave a note to ourselves so that we know not to decline in the
    * map_to_storage hook. */

If you add the option -x-p to that command you'll get:

Index: subversion/mod_dav_svn/mod_dav_svn.c
===================================================================
--- subversion/mod_dav_svn/mod_dav_svn.c    (revision 1544710)
+++ subversion/mod_dav_svn/mod_dav_svn.c    (revision 1544711)
@@ -1097,7 +1097,8 @@ static int dav_svn__handler(request_rec *r)

 /* Fill the filename on the request with a bogus path since we aren't serving
  * a file off the disk.  This means that <Directory> blocks will not match and
- * that %f in logging formats will show as "svn:/path/to/repo/path/in/repo". */
+ * %f in logging formats will show as "dav_svn:/path/to/repo/path/in/repo".
+ */
 static int dav_svn__translate_name(request_rec *r)
 {
   const char *fs_path, *repos_basename, *repos_path;
@@ -1146,7 +1147,7 @@ static int dav_svn__translate_name(request_rec *r)
   if (repos_path && '/' == repos_path[0] && '\0' == repos_path[1])
     repos_path = NULL;

-  /* Combine 'svn:', fs_path and repos_path to produce the bogus path we're
+  /* Combine 'dav_svn:', fs_path and repos_path to produce the bogus path we're
    * placing in r->filename.  We can't use our standard join helpers such
    * as svn_dirent_join.  fs_path is a dirent and repos_path is a fspath
    * (that can be trivially converted to a relpath by skipping the leading
@@ -1154,7 +1155,7 @@ static int dav_svn__translate_name(request_rec *r)
    * repository is 'trunk/c:hi' this results in a non canonical dirent on
    * Windows. Instead we just cat them together. */
   r->filename = apr_pstrcat(r->pool,
-                            "svn:", fs_path, repos_path, SVN_VA_NULL);
+                            "dav_svn:", fs_path, repos_path, SVN_VA_NULL);

   /* Leave a note to ourselves so that we know not to decline in the
    * map_to_storage hook. */

Note how the function is added after the @@ on the range lines. This portion of the lines are ignored by any software processing the diff. So you're free to put whatever you want there. You could put your comments there.

Unidiff hunks start each line with ' ' (space) to mean context (as in an unchanged line), '+' to mean an added line, or '-' to mean a removed line. A lot of parsers will (including Subversion's svn patch command) will discard lines that start with some other character. So you might be able to simply insert a line that starts with some other character. But that's not guaranteed to be as portable as the above method.