> I don't think one can understand what the feature does just by reading
> the doc string.  I think something very basic is missing here, without
> which the rest of the doc text cannot be unlocked.  Perhaps just
> elaborating on what exactly "syntax highlighting" means in this
> context would be enough.
>
> Also, judging by my reading of the code, the description of what the
> various non-nil values do is not entirely accurate, and might not be
> what the user expects by reading the above description.

I tried to explain this more thoughtfully in a new version of the docstring.

>> +                 (const :tag "Without full source or get it from files" t)))
>
> This description is backwards, I think: you should start with "with
> source files".  (But maybe I misunderstand the whole issue, see
> above.)

Fixed in a new version.  Also please note why `t' is not the default.
This is to avoid trying to read local files while a received patch
is displayed in the mail attachment.  I think to avoid such situations
when it will try to read random files, better to use the same method
as is used in diff buffers created by a VC command - it sets a special
variable `diff-vc-backend' that guarantees that diff buffer is created
with file paths relative to the process that created these buffers.
I propose for commands that compare files like diff, diff-backup,
dired-diff, dired-backup-diff also to set a similar variable e.g.
`diff-files' that will guarantee that file paths are valid to read.

>> +                                      ;; Restore restore previous window configuration
>> +                                      ;; because when vc-find-revision can't find a revision
>> +                                      ;; (e.g. for /dev/null), it jumps to another window
>> +                                      ;; using pop-to-buffer in vc-do-command when
>> +                                      ;; the buffer name doesn't begin with a space char.
>
> Nitpicking: can this comment please be refilled to not exceed "normal"
> line width?

Fixing the described problem will remove this comment,
but I have no idea how better to do this.  The problem is that
we need to provide own created buffer to the call to `vc-find-revision'.
Currently it has the following function signature:

  (defun vc-find-revision (file revision &optional backend)

But VC API in the comments in the beginning of vc.el
is documented with a different function signature:

  ;; * find-revision (file rev buffer)
  ;;
  ;;   Fetch revision REV of file FILE and put it into BUFFER.
  ;;   If REV is the empty string, fetch the head of the trunk.
  ;;   The implementation should pass the value of vc-checkout-switches
  ;;   to the backend command.

This means that to fix the problem we need the call as is documented
with the argument BUFFER, but the current implementation without
such argument doesn't correspond to the documentation.

BTW, while deciding what to do with this, could you please confirm
if I correctly fixed another problem in vc-find-revision-no-save.
Recently in bug#33319 I added this function but now discovered
a problem with encoding.  A vc process outputs lines to the buffer
with no-conversion, so in the patch below I added recode-region
to convert output to the buffer's encoding.  coding-system-for-write
that I removed was copied from vc-find-revision-save where is was
needed for write-region called from the macro with-temp-file,
but vc-find-revision-no-save doesn't write output to the file.

>> +     ((not (eq diff-font-lock-syntax 'vc))
>> +      (let ((file (car (diff-hunk-file-names old))))
>> +        (if (and file (file-exists-p file))
>
> This assumes that the file name is relative to the default-directory
> of the buffer with the diffs, right?  How reasonable is such an
> assumption for when browsing diffs?  Should we perhaps allow the user
> to specify the directory of the sources?

This assumption should be true for all cases when the diff buffer is created
using commands like dired-diff, dired-backup-diff, diff, diff-backup.

But when navigating a diff output saved to a file that was moved to
another directory, currently diff-mode asks for a directory interactively,
that is not possible to do for non-interactive fontification.

As a general solution is should be possible to specify the default
directory in the local variables at the first line of the diff files
as currently already is used in compilation/grep buffers like

-*- mode: diff-mode; default-directory: "..." -*-

> Also, if the diffs are from Git, they begin with a/, b/, etc. dummy
> directories, which usually don't exist in the file system.

This is not a problem because diff-find-file-name used in the patch
strips such a/, b/ prefixes to get the existing file name.