Subject: bug#33567: Syntactic fontification of diff hunks

From: Eli Zaretskii <[email-masked]>
Date: Wed, 05 Dec 2018 09:25:22 +0200

> From: Juri Linkov <[email-masked]>
> Cc: [email-masked]
> Date: Wed, 05 Dec 2018 01:28:45 +0200
> 
> >> For more safety, I propose to set a new buffer-local variable
> >> `diff-default-directory' by such commands as diff, diff-backup,
> >> dired-diff, dired-backup-diff.  The existence of such variable
> >> should guarantee that the referenced files really exist.
> >> This variable will be like `diff-vc-backend' that says that
> >> the diff-mode buffer is created by the VCS command.
> >> Then anyone who want to visit a diff file in another directory,
> >> could add it to the first line:
> >>
> >> -*- mode: diff-mode; diff-default-directory: "..." -*-
> >
> > I'm not sure this is a step in the right direction.  What is the
> > advantage of having a separate variable?  How is it "safer"?
> 
> When this special variable is set by a diff command, it's safe to assume
> that the files referenced from the diff buffer really exist, so it's
> safe to try reading them.  I don't want a patch in a mail attachment
> to try reading files mentioned in the patch attachment.

I think you lean heavily towards the use case where the diffs were
produced by some Emacs command.  Whereas what I have in mind is the
use case where the diffs came from some other source, like manual
invocation of shell commands.  I'm saying that in the absence of
diff-default-directory, using default-directory and 'cd' will be much
easier for users.

> >> This means that working revisions can't be extracted from the repository.
> >> Until committed, they reside in files that are visited with find-file.
> >
> > We need to describe the implications of that to the users.  Does the
> > following text capture the issue?
> >
> >   For diffs against the working-tree version of a file, the
> >   highlighting is based on the current file contents, which could be
> >   different from the contents when the diffs were taken.  In such
> >   cases, the produced highlighting might be wrong.
> 
> Such problem is very rare because highlighting is added usually
> immediately after creating a diff.  When the file contents changes,
> there is no highlighting at all - it verifies if text of the hunk
> exist in the file, so highlighting never is wrong.

The use case I have in mind is that some time passes between the
generation of the diffs and the time the diffs are visited and
fontified.  During that time, the working revision could have been
changed.  Isn't that what this issue is about?  If so, we should
explain why in that case, rare as it could be, the fontification might
be wrong.

> >> > Also, does it mean `vc' will not fall back to `hunk-only'?  Why not?
> >>
> >> Actually, it already falls back to `hunk-only', this is what
> >> "on failure get fontification from hunk alone." tries to say.
> >
> > There's no such text in the description of 'vc', only in the
> > description of t, which is why I asked.
> 
> Maybe then better to add text common for all cases, e.g.
> 
> "If some method fails, get fontification from hunk alone."

That'd be fine as well.

Thanks.