GNU bug report logs - #33567
Syntactic fontification of diff hunks

Previous Next

Full log

View this message in rfc822 format

From: Juri Linkov <juri <at> linkov.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 33567 <at> debbugs.gnu.org
Subject: bug#33567: Syntactic fontification of diff hunks
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.

>> >> > 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.
>> >
>> > Not in my testing, but maybe I tried in the wrong Emacs version.  Is
>> > this feature new with Emacs 27?
>>
>> For testing better try to eval e.g. `(diff-find-file-name nil t)'
>> on a hunk in a diff-mode buffer created by git.
>
> I did, but I guess this must be done inside the repository to work,
> does it?  If I put the output of "git diff" on a file in some
> arbitrary directory, then visit that file and evaluate
> (diff-find-file-name nil t), I get nil.

Yes, it finds only the existing files inside the repository.

>> >> +For working revisions get highlighting according to the working
>> >> +copy of the file.
>> >
>> > I don't understand the significance of this comment.  If you want to
>> > say that the produced highlighting might be wrong if the working
>> > version has changed since it was compared, then let's say that
>> > explicitly.
>>
>> 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.

>> >> +If t, additionally to trying to use a version control system to get
>> >> +old revisions for fontification, also try to get fontification based
>> >> +on existing files, and on failure get fontification from hunk alone."
>> >
>> > What is the difference between using a VCS to get old revisions, and
>> > using existing files?
>>
>> This means that when a diff-mode buffer is not created by a VCS,
>> then it tries to read files with find-file.
>
> If so, I suggest the following wording:
>
>   If t, try to infer fontification from the compared files, if they
>   exist in the filesystem, when accessing their contents via VC
>   fails.

Will add in the final patch.

>> > 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."

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.