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

Previous Next

Full log

Message #80 received at 33567 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Juri Linkov <juri <at> linkov.net>
Cc: 33567 <at> debbugs.gnu.org
Subject: Re: bug#33567: Syntactic fontification of diff hunks
Date: Fri, 14 Dec 2018 11:13:48 +0200

> From: Juri Linkov <juri <at> linkov.net>
> Cc: 33567 <at> debbugs.gnu.org
> Date: Thu, 13 Dec 2018 01:17:52 +0200
> 
> Here is a more less final version that I tested on many different diffs:

Thanks, I have a few minor comments, mainly to the documentation
parts.

> diff --git a/etc/NEWS b/etc/NEWS
> index decc5e3954..1024e9e9f5 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -273,6 +273,8 @@ To disable it, set the new defcustom 'diff-font-lock-refine' to nil.
>  *** File headers can be shortened, mimicking Magit's diff format
>  To enable it, set the new defcustom 'diff-font-lock-prettify to t.
>  
> +*** Source language syntax is highlighted in diff hunks when 'diff-font-lock-syntax' is t.

This line is too long, suggest to rephrase:

  *** Better syntax highlighting of Diff hunks.
  Fragments of source in Diff hunks are now by default highlighted
  according to the appropriate major mode.  Customize the new option
  'diff-font-lock-syntax' to nil to disable this.

> +@vindex diff-font-lock-syntax
> +  If non-nil, diff hunk font-lock includes source language syntax highlighting.

"nil" should be in @code.

> diff --git a/lisp/vc/diff.el b/lisp/vc/diff.el
> index ac94586cac..93fa54b1fb 100644
> --- a/lisp/vc/diff.el
> +++ b/lisp/vc/diff.el
> @@ -165,6 +167,7 @@ diff-no-select
>             (lambda (_ignore-auto _noconfirm)
>               (diff-no-select old new switches no-async (current-buffer))))
>        (setq default-directory thisdir)
> +      (set (make-local-variable 'diff-default-directory) default-directory)

Any reason not to use setq-local?

> +(defun diff--font-lock-syntax (max)
> +  "Syntax highlighting from font-lock."

Although an internal function, could it have a slightly more detailed
do string, please?

> +(defun diff-syntax-fontify-hunk (beg end old)
> +  "Highlight source language syntax in diff hunks."

This is for a single hunk, not "hunks", right?  Also, please mention
the arguments in the doc string.

> +(defun diff-syntax-fontify-props (file text line-nb &optional no-init hunk-only)
> +  "Get font-lock properties from the source code."

Please mention the arguments ion the doc string.

Previous Next

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