bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

[-- Attachment #1: Type: text/plain, Size: 258 bytes --]


When `diff-font-lock-prettify' is `t', outline headers aren't displayed
at the beginning of a line.

The following screenshot displays a
*vc-diff* buffer with outline minor mode enabled and body hidden with
and without `diff-font-lock-prettify' being `t':


[-- Attachment #2: Capture d’écran du 2021-10-04 22-50-45.png --]
[-- Type: image/png, Size: 110367 bytes --]

[-- Attachment #3: Type: text/plain, Size: 6904 bytes --]


Recipe to reproduce:

1. Set `diff-font-lock-prettify' to `t' and enable `outline-minor-mode'
in `diff-mode'; For example:

(add-hook 'diff-mode-hook
	  (lambda ()
            (setq diff-font-lock-prettify t)
	    (outline-minor-mode)))

2. In a *vc-change-log* buffer for emacs sources, find the commit with
hash a8de88e330. Press = (`log-view').

3. Press C-c @ C-t (`outline-hide-body') while in the *vc-diff*
buffer. Observe that outline headers are splitted in multiple lines.


In GNU Emacs 28.0.50 (build 2, x86_64-pc-linux-gnu, GTK+ Version 3.24.24, cairo version 1.16.0)
 of 2021-09-13 built on carbon
Repository revision: 8c80430824d52e5dfae97db2669b419621817956
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000
System Description: Debian GNU/Linux 11 (bullseye)

Configured using:
 'configure --with-native-compilation'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 M17N_FLT MODULES NATIVE_COMP NOTIFY
INOTIFY PDUMPER PNG RSVG SECCOMP SOUND THREADS TIFF TOOLKIT_SCROLL_BARS
X11 XDBE XIM XPM GTK3 ZLIB

Important settings:
  value of $LANG: fr_FR.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Message

Minor modes in effect:
  goto-address-mode: t
  highlight-changes-visible-mode: t
  show-paren-mode: t
  shell-dirtrack-mode: t
  minions-mode: t
  mml-mode: t
  desktop-save-mode: t
  save-place-mode: t
  electric-pair-mode: t
  icomplete-mode: t
  global-so-long-mode: t
  global-auto-revert-mode: t
  auto-insert-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  electric-layout-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  window-divider-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  line-number-mode: t
  auto-fill-function: message-do-auto-fill
  transient-mark-mode: t
  abbrev-mode: t

Load-path shadows:
/home/matthias/.config/emacs/elpa/transient-20210919.1006/transient hides /usr/local/share/emacs/28.0.50/lisp/transient
/home/matthias/.config/emacs/elpa/flymake-1.2.1/flymake hides /usr/local/share/emacs/28.0.50/lisp/progmodes/flymake
/home/matthias/.config/emacs/elpa/project-0.7.1/project hides /usr/local/share/emacs/28.0.50/lisp/progmodes/project
/home/matthias/.config/emacs/elpa/dictionary-20201001.1727/dictionary hides /usr/local/share/emacs/28.0.50/lisp/net/dictionary

Features:
(shadow emacsbug sendmail nndoc gnus-dup url-cache crm debbugs-gnu
debbugs soap-client url-http url-auth url-gw rng-xsd xsd-regexp
debbugs-browse dabbrev cl-print cus-start texinfo texinfo-loaddefs
help-fns radix-tree js add-log log-view pcvs-util misearch multi-isearch
smerge-mode diff gnus-fun flow-fill mm-archive view sort smiley
gnus-cite mail-extr gnus-async gnus-bcklg qp gnus-ml disp-table
gnus-topic nndraft nnmh nnfolder utf-7 epa-file gnutls network-stream
nsm gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg
gnus-cache hl-line conf-mode whitespace ox-odt rng-loc rng-uri rng-parse
rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok
nxml-util ox-latex ox-icalendar ox-html table ox-ascii ox-publish ox
org-element avl-tree generator ol-eww eww xdg url-queue mm-url ol-rmail
ol-mhe ol-irc ol-info ol-gnus nnselect gnus-search eieio-opt speedbar
ezimage dframe gnus-art mm-uu mml2015 mm-view mml-smime smime dig
gnus-sum shr kinsoku svg dom ol-docview doc-view image-mode exif
ol-bibtex bibtex ol-bbdb ol-w3m flyspell goto-addr bug-reference
dired-aux vc-dir follow mule-util display-line-numbers hilit-chg paren
vc-mtn vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs vc jka-compr
bash-completion shell eglot array jsonrpc ert ewoc debug backtrace xref
flymake-proc flymake compile pcase project imenu avoid minions
carbon-custom cus-edit cus-load gnus-demon nntp gnus-group gnus-undo
gnus-start gnus-dbus dbus xml gnus-cloud nnimap nnmail mail-source utf7
netrc parse-time iso8601 gnus-spec gnus-win nnoo gnus-int gnus-range
message rmc puny rfc822 mml mml-sec epa derived epg rfc6068 epg-config
mm-decode mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils
mailheader gnus nnheader gnus-util rmail rmail-loaddefs rfc2047 rfc2045
ietf-drums mail-utils mm-util mail-prsvr wid-edit gnus-dired dired-x
dired dired-loaddefs org-capture org-refile org ob ob-tangle ob-ref
ob-lob ob-table ob-exp org-macro org-footnote org-src ob-comint
org-pcomplete pcomplete comint ansi-color ring org-list org-faces
org-entities time-date org-version ob-emacs-lisp ob-core ob-eval
org-table ol org-keys org-compat org-macs org-loaddefs format-spec
find-func cal-menu calendar cal-loaddefs dictionary link connection
advice markdown-mode edit-indirect color thingatpt noutline outline
skeleton find-file vc-git diff-mode easy-mmode vc-dispatcher ispell
desktop frameset server bookmark text-property-search pp saveplace
elec-pair edmacro kmacro icomplete so-long autorevert filenotify
autoinsert cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles cc-align
cc-engine cc-vars cc-defs generic-x face-remap proof-site
proof-autoloads comp comp-cstr warnings rx cl-extra help-mode info
package browse-url url url-proxy url-privacy url-expand url-methods
url-history url-cookie url-domsuf url-util mailcap url-handlers
url-parse auth-source cl-seq eieio eieio-core cl-macs eieio-loaddefs
password-cache json subr-x map url-vars seq byte-opt gv bytecomp
byte-compile cconv cl-loaddefs cl-lib iso-transl tooltip eldoc electric
uniquify ediff-hook vc-hooks lisp-float-type mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode elisp-mode lisp-mode
prog-mode register page tab-bar menu-bar rfn-eshadow isearch easymenu
timer select scroll-bar mouse jit-lock font-lock syntax font-core
term/tty-colors frame minibuffer cl-generic cham georgian utf-8-lang
misc-lang vietnamese tibetan thai tai-viet lao korean japanese eucjp-ms
cp51932 hebrew greek romanian slovak czech european ethiopic indian
cyrillic chinese composite charscript charprop case-table epa-hook
jka-cmpr-hook help simple abbrev obarray cl-preloaded nadvice button
loaddefs faces cus-face macroexp files window text-properties overlay
sha1 md5 base64 format env code-pages mule custom widget
hashtable-print-readable backquote threads dbusbind inotify lcms2
dynamic-setting system-font-setting font-render-setting cairo
move-toolbar gtk x-toolkit x multi-tty make-network-process
native-compile emacs)

Memory information:
((conses 16 1700523 255710)
 (symbols 48 38830 103)
 (strings 32 227548 67913)
 (string-bytes 1 7153152)
 (vectors 16 75900)
 (vector-slots 8 1725779 101458)
 (floats 8 2990 1100)
 (intervals 56 136558 2364)
 (buffers 992 150))

-- 
Matthias

bug#51016: Status: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

tags 51016 + patch
quit

bug#51016: Status: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: 0001-Fix-outline-headers-of-prettified-diff-when-using-Gi.patch --]
[-- Type: text/x-diff, Size: 4209 bytes --]

From 8e07d5518ed49eef95e21bd94bc93ce3d79f6962 Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Sat, 9 Oct 2021 00:36:11 +0200
Subject: [PATCH] Fix outline headers of prettified diff when using Git
 (bug#51016)

* lisp/vc/diff-mode.el (diff-outline-level): Make hunk headers be at level 2
(diff-mode): Use prettified line as outline header
* lisp/vc/vc.el (vc-diff-internal): Fix diff mode being set before content
inserted
---
 lisp/vc/diff-mode.el | 28 ++++++++++++++++++++--------
 lisp/vc/vc.el        |  5 +++--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 057ffcd06e..fd4b2b15df 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -894,6 +894,10 @@ diff-split-hunk
       ;; Fix the original hunk-header.
       (diff-fixup-modifs start pos))))
 
+(defun diff-outline-level ()
+  (if (string-match-p diff-hunk-header-re (match-string 0))
+      2 1))
+
 
 ;;;;
 ;;;; jump to other buffers
@@ -1494,7 +1498,6 @@ diff-mode
 
   (setq-local font-lock-defaults diff-font-lock-defaults)
   (add-hook 'font-lock-mode-hook #'diff--font-lock-cleanup nil 'local)
-  (setq-local outline-regexp diff-outline-regexp)
   (setq-local imenu-generic-expression
               diff-imenu-generic-expression)
   ;; These are not perfect.  They would be better done separately for
@@ -1543,7 +1546,17 @@ diff-mode
     (setq-local diff-buffer-type
                 (if (re-search-forward "^diff --git" nil t)
                     'git
-                  nil))))
+                  nil)))
+  (when (and (eq diff-buffer-type 'git) diff-font-lock-prettify)
+    (setq diff-outline-regexp
+          (concat
+           "\\("
+           "^diff --git.*\n\\(--- .+\n\\+\\+\\+ \\|\\*\\*\\* .+\n--- \\|[^-+!<>0-9@* \n]\\).+\n"
+           "\\|"
+           diff-hunk-header-re
+           "\\)"))
+    (setq-local outline-level #'diff-outline-level))
+  (setq-local outline-regexp diff-outline-regexp))
 
 ;;;###autoload
 (define-minor-mode diff-minor-mode
@@ -2603,13 +2616,12 @@ diff--font-lock-prettify
                            (or (match-beginning 2) (match-beginning 1))
                            'display (propertize
                                      (cond
-                                      ((null (match-beginning 1)) "new file  ")
-                                      ((null (match-beginning 2)) "deleted   ")
-                                      (t                          "modified  "))
+                                      ((null (match-beginning 1)) (concat "new file  " (match-string 2)))
+                                      ((null (match-beginning 2)) (concat "deleted   " (match-string 1)))
+                                      (t                          (concat "modified  " (match-string 1))))
                                      'face '(diff-file-header diff-header)))
-        (unless (match-beginning 2)
-          (put-text-property (match-end 1) (1- (match-end 0))
-                             'display "")))))
+        (put-text-property (match-end 1) (1- (match-end 0))
+                           'display ""))))
   nil)
 
 ;;; Syntax highlighting from font-lock
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index edc4169465..42b0a1b986 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1793,7 +1793,6 @@ vc-diff-internal
         (setq files (nreverse filtered))))
     (vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer async)
     (set-buffer buffer)
-    (diff-mode)
     (setq-local diff-vc-backend (car vc-fileset))
     (setq-local diff-vc-revisions (list rev1 rev2))
     (setq-local revert-buffer-function
@@ -1815,7 +1814,9 @@ vc-diff-internal
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
-        (vc-run-delayed (vc-diff-finish buf (when verbose messages))))
+        (vc-run-delayed (progn
+                          (vc-diff-finish buf (when verbose messages))
+                          (diff-mode))))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))
-- 
2.30.2

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

[-- Attachment #1: Type: text/plain, Size: 145 bytes --]

I updated the patch after a week using it. Outlines headers are now
updated for diff comming from Git even when diff-font-lock-prettify is
nil.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-outline-headers-of-diff-buffers-when-using-Git-b.patch --]
[-- Type: text/x-diff, Size: 4104 bytes --]

From 67ef81b503b2529e28fcdca35912bf5c9ce54d8e Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Sat, 9 Oct 2021 00:36:11 +0200
Subject: [PATCH] Fix outline headers of diff buffers when using Git
 (bug#51016)

* lisp/vc/diff-mode.el (diff-outline-level): Make hunk headers be at level 2
(diff-mode): Use prettified line as outline header
* lisp/vc/vc.el (vc-diff-internal): Fix diff mode being set before content
inserted
---
 lisp/vc/diff-mode.el | 28 ++++++++++++++++++++--------
 lisp/vc/vc.el        |  5 +++--
 2 files changed, 23 insertions(+), 10 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 057ffcd06e..f0a6cb7b15 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -894,6 +894,10 @@ diff-split-hunk
       ;; Fix the original hunk-header.
       (diff-fixup-modifs start pos))))
 
+(defun diff-outline-level ()
+  (if (string-match-p diff-hunk-header-re (match-string 0))
+      2 1))
+
 
 ;;;;
 ;;;; jump to other buffers
@@ -1494,7 +1498,6 @@ diff-mode
 
   (setq-local font-lock-defaults diff-font-lock-defaults)
   (add-hook 'font-lock-mode-hook #'diff--font-lock-cleanup nil 'local)
-  (setq-local outline-regexp diff-outline-regexp)
   (setq-local imenu-generic-expression
               diff-imenu-generic-expression)
   ;; These are not perfect.  They would be better done separately for
@@ -1543,7 +1546,17 @@ diff-mode
     (setq-local diff-buffer-type
                 (if (re-search-forward "^diff --git" nil t)
                     'git
-                  nil))))
+                  nil)))
+  (when (eq diff-buffer-type 'git)
+    (setq diff-outline-regexp
+          (concat
+           "\\("
+           "^diff --git.*\n"
+           "\\|"
+           diff-hunk-header-re
+           "\\)"))
+    (setq-local outline-level #'diff-outline-level))
+  (setq-local outline-regexp diff-outline-regexp))
 
 ;;;###autoload
 (define-minor-mode diff-minor-mode
@@ -2603,13 +2616,12 @@ diff--font-lock-prettify
                            (or (match-beginning 2) (match-beginning 1))
                            'display (propertize
                                      (cond
-                                      ((null (match-beginning 1)) "new file  ")
-                                      ((null (match-beginning 2)) "deleted   ")
-                                      (t                          "modified  "))
+                                      ((null (match-beginning 1)) (concat "new file  " (match-string 2)))
+                                      ((null (match-beginning 2)) (concat "deleted   " (match-string 1)))
+                                      (t                          (concat "modified  " (match-string 1))))
                                      'face '(diff-file-header diff-header)))
-        (unless (match-beginning 2)
-          (put-text-property (match-end 1) (1- (match-end 0))
-                             'display "")))))
+        (put-text-property (match-end 1) (1- (match-end 0))
+                           'display ""))))
   nil)
 
 ;;; Syntax highlighting from font-lock
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index 5b259fcdb3..c82110274b 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1793,7 +1793,6 @@ vc-diff-internal
         (setq files (nreverse filtered))))
     (vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer async)
     (set-buffer buffer)
-    (diff-mode)
     (setq-local diff-vc-backend (car vc-fileset))
     (setq-local diff-vc-revisions (list rev1 rev2))
     (setq-local revert-buffer-function
@@ -1815,7 +1814,9 @@ vc-diff-internal
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
-        (vc-run-delayed (vc-diff-finish buf (when verbose messages))))
+        (vc-run-delayed (progn
+                          (vc-diff-finish buf (when verbose messages))
+                          (diff-mode))))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))
-- 
2.30.2

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Matthias Meulien <orontee@gmail.com> writes:

> I updated the patch after a week using it. Outlines headers are now
> updated for diff comming from Git even when diff-font-lock-prettify is
> nil.

Thanks for the patch and the thorough case for reproduction.  I've now
applied it to Emacs 29 (with some trivial changes).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> I updated the patch after a week using it. Outlines headers are now
>> updated for diff comming from Git even when diff-font-lock-prettify is
>> nil.
>
> Thanks for the patch and the thorough case for reproduction.  I've now
> applied it to Emacs 29 (with some trivial changes).

This patch broke vc-diff.  For example, after 'C-x v L' type 'd',
and check that *vc-diff* buffer doesn't have buffer-local variables
diff-vc-backend, diff-vc-revisions anymore, because they were defined
in vc-diff-internal after diff-mode, but now the postponed diff-mode
kills these buffer-local variables:

     (set-buffer buffer)
-    (diff-mode)
     (setq-local diff-vc-backend (car vc-fileset))
     (setq-local diff-vc-revisions (list rev1 rev2))

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> This patch broke vc-diff.  For example, after 'C-x v L' type 'd',
> and check that *vc-diff* buffer doesn't have buffer-local variables
> diff-vc-backend, diff-vc-revisions anymore, because they were defined
> in vc-diff-internal after diff-mode, but now the postponed diff-mode
> kills these buffer-local variables:
>
>      (set-buffer buffer)
> -    (diff-mode)
>      (setq-local diff-vc-backend (car vc-fileset))
>      (setq-local diff-vc-revisions (list rev1 rev2))

Right.  Matthias -- can this be fixed in a different way that doesn't
postpone changing the major mode?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

[-- Attachment #1: Type: text/plain, Size: 1269 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Juri Linkov <juri@linkov.net> writes:
>
>> This patch broke vc-diff.  For example, after 'C-x v L' type 'd',
>> and check that *vc-diff* buffer doesn't have buffer-local variables
>> diff-vc-backend, diff-vc-revisions anymore, because they were defined
>> in vc-diff-internal after diff-mode, but now the postponed diff-mode
>> kills these buffer-local variables:
>>
>>      (set-buffer buffer)
>> -    (diff-mode)
>>      (setq-local diff-vc-backend (car vc-fileset))
>>      (setq-local diff-vc-revisions (list rev1 rev2))
>
> Right.  Matthias -- can this be fixed in a different way that doesn't
> postpone changing the major mode?

Thanks Juri!

I moved the call to `diff-mode` since in its implementation one
initialize `diff-buffer-type` by searching:

  (save-excursion
    (setq-local diff-buffer-type
                (if (re-search-forward "^diff --git" nil t)
                    'git
                  nil)))

I suggest to restore the original call to `diff-mode` (before the
asynchronous operation completes) and keep the initialization of
`diff-buffer-type` by searching but make it available to
`vc-diff-finish' which is called when the asynchronous operation
completes.

See the attached patch for details.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-local-variables-overwritten-when-diff-mode-is-se.patch --]
[-- Type: text/x-diff, Size: 3612 bytes --]

From 61d0a96df15f3fe4c44ade01747246b05fed2a48 Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Mon, 8 Nov 2021 23:37:33 +0100
Subject: [PATCH] Fix local variables overwritten when diff-mode is set

* lisp/vc/diff-mode.el (diff-mode):
(diff-setup-buffer-type): Defun to initialize `diff-buffer-type'

* lisp/vc/vc.el (vc-diff-finish): Set `diff-buffer-type' after content
inserted
(vc-diff-internal): Restore `diff-mode' being set before local variables
---
 lisp/vc/diff-mode.el | 26 ++++++++++++++++----------
 lisp/vc/vc.el        |  6 +++---
 2 files changed, 19 insertions(+), 13 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 746f76b46c..e68aa2257d 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1540,16 +1540,7 @@ diff-mode
                 #'diff--filter-substring)
   (unless buffer-file-name
     (hack-dir-local-variables-non-file-buffer))
-  (save-excursion
-    (setq-local diff-buffer-type
-                (if (re-search-forward "^diff --git" nil t)
-                    'git
-                  nil)))
-  (when (eq diff-buffer-type 'git)
-    (setq diff-outline-regexp
-          (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
-    (setq-local outline-level #'diff--outline-level))
-  (setq-local outline-regexp diff-outline-regexp))
+  (diff-setup-buffer-type))
 
 ;;;###autoload
 (define-minor-mode diff-minor-mode
@@ -1585,6 +1576,21 @@ diff-setup-whitespace
                     "^[-+!] .*?\\([\t ]+\\)$"
                   "^[-+!<>].*?\\([\t ]+\\)$"))))
 
+(defun diff-setup-buffer-type ()
+  "Try to guess the `diff-buffer-type' from content of current Diff mode buffer.
+`outline-regexp' is updated accordingly."
+  (save-excursion
+    (goto-char (point-min))
+    (setq-local diff-buffer-type
+                (if (re-search-forward "^diff --git" nil t)
+                    'git
+                  nil)))
+  (when (eq diff-buffer-type 'git)
+    (setq diff-outline-regexp
+          (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
+    (setq-local outline-level #'diff--outline-level))
+  (setq-local outline-regexp diff-outline-regexp))
+
 (defun diff-delete-if-empty ()
   ;; An empty diff file means there's no more diffs to integrate, so we
   ;; can just remove the file altogether.  Very handy for .rej files if we
diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
index c9500f454a..87137d8ede 100644
--- a/lisp/vc/vc.el
+++ b/lisp/vc/vc.el
@@ -1739,6 +1739,7 @@ vc-diff-finish
 	       (insert (cdr messages) ".\n")
 	       (message "%s" (cdr messages))))
 	(diff-setup-whitespace)
+        (diff-setup-buffer-type)
 	(goto-char (point-min))
 	(when window
 	  (shrink-window-if-larger-than-buffer window)))
@@ -1804,6 +1805,7 @@ vc-diff-internal
         (setq files (nreverse filtered))))
     (vc-call-backend (car vc-fileset) 'diff files rev1 rev2 buffer async)
     (set-buffer buffer)
+    (diff-mode)
     (setq-local diff-vc-backend (car vc-fileset))
     (setq-local diff-vc-revisions (list rev1 rev2))
     (setq-local revert-buffer-function
@@ -1825,9 +1827,7 @@ vc-diff-internal
       ;; after `pop-to-buffer'; the former assumes the diff buffer is
       ;; shown in some window.
       (let ((buf (current-buffer)))
-        (vc-run-delayed (progn
-                          (vc-diff-finish buf (when verbose messages))
-                          (diff-mode))))
+        (vc-run-delayed (vc-diff-finish buf (when verbose messages))))
       ;; In the async case, we return t even if there are no differences
       ;; because we don't know that yet.
       t)))
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
Matthias

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Matthias Meulien <orontee@gmail.com> writes:

> I suggest to restore the original call to `diff-mode` (before the
> asynchronous operation completes) and keep the initialization of
> `diff-buffer-type` by searching but make it available to
> `vc-diff-finish' which is called when the asynchronous operation
> completes.
>
> See the attached patch for details.

Thanks; this fixes the problem reported by Juri (as far as I can see
after doing some light testing), so I've now pushed the patch to Emacs
29.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> I suggest to restore the original call to `diff-mode` (before the
>> asynchronous operation completes) and keep the initialization of
>> `diff-buffer-type` by searching but make it available to
>> `vc-diff-finish' which is called when the asynchronous operation
>> completes.
>>
>> See the attached patch for details.
>
> Thanks; this fixes the problem reported by Juri (as far as I can see
> after doing some light testing), so I've now pushed the patch to Emacs
> 29.

I confirm it's fixed (Matthias placed two counterpart functions together:
diff-setup-whitespace and diff-setup-buffer-type in the patch,
so I restored the same in master for readability).

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> I updated the patch after a week using it. Outlines headers are now
>> updated for diff comming from Git even when diff-font-lock-prettify is
>> nil.
>
> Thanks for the patch and the thorough case for reproduction.  I've now
> applied it to Emacs 29 (with some trivial changes).

As noticed in bug#51809, outline-cycle is not bound on hunk headings anymore,
only on file headings.  It seems the problem is somewhere here:

    (setq diff-outline-regexp
          (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))

But I don't know why diff-hunk-header-re doesn't match hunk headings
in outline-minor-mode.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> As noticed in bug#51809, outline-cycle is not bound on hunk headings anymore,
> only on file headings.  It seems the problem is somewhere here:
>
>     (setq diff-outline-regexp
>           (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
>
> But I don't know why diff-hunk-header-re doesn't match hunk headings
> in outline-minor-mode.

Hm...  I just tried switching outline-minor-mode on and
(re-search-forward diff-hunk-header-re), and it seemed to work as
expected?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

> Juri Linkov <juri@linkov.net> writes:
>
>> As noticed in bug#51809, outline-cycle is not bound on hunk headings anymore,
>> only on file headings.  It seems the problem is somewhere here:
>>
>>     (setq diff-outline-regexp
>>           (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
>>
>> But I don't know why diff-hunk-header-re doesn't match hunk headings
>> in outline-minor-mode.
>
> Hm...  I just tried switching outline-minor-mode on and
> (re-search-forward diff-hunk-header-re), and it seemed to work as
> expected?

Yep, I already tried `(re-search-forward diff-hunk-header-re)' too,
and it works.  But still typing TAB on the hunk heading
with outline-minor-mode-cycle=t does nothing, whereas typing TAB
on the file heading correctly cycles the file diff.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Yep, I already tried `(re-search-forward diff-hunk-header-re)' too,
> and it works.  But still typing TAB on the hunk heading
> with outline-minor-mode-cycle=t does nothing, whereas typing TAB
> on the file heading correctly cycles the file diff.

Oh yeah, now I'm seeing the problem.  I don't really use outline mode
daily, so I'm not sure how it's supposed to work, but -- TAB in these
buffers are bound to diff-hunk-next, and as far as I can tell, outline
mode has just placed a keymap property on the file headings, not the
hunk headings.

Or is that the actual bug here?  😀  I should probably have read the
entire thread before getting involved.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> Yep, I already tried `(re-search-forward diff-hunk-header-re)' too,
>> and it works.  But still typing TAB on the hunk heading
>> with outline-minor-mode-cycle=t does nothing, whereas typing TAB
>> on the file heading correctly cycles the file diff.
>
> Oh yeah, now I'm seeing the problem.  I don't really use outline mode
> daily, so I'm not sure how it's supposed to work, but -- TAB in these
> buffers are bound to diff-hunk-next, and as far as I can tell, outline
> mode has just placed a keymap property on the file headings, not the
> hunk headings.
>
> Or is that the actual bug here?  😀  I should probably have read the
> entire thread before getting involved.

Actually, ‘outline-font-lock-keywords’ extends the outline regexp
to (concat "^\\(?:" outline-regexp "\\).+").  And indeed,

  (re-search-forward (concat "^\\(?:" diff-hunk-header-re "\\).+"))

fails in diff buffers.  Don't ask me why outline.el adds ".+"
at the end, I don't know 🤐  To circumvent this idiosyncrasy
for headings in ‘describe-bindings’, I had to use a different regexp
in ‘outline-minor-mode-highlight-buffer’ (not based on font-lock):

  (concat "^\\(?:" outline-regexp "\\).*$")))

where ".+" is replaced with less-strict ".*".

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> Actually, ‘outline-font-lock-keywords’ extends the outline regexp
> to (concat "^\\(?:" outline-regexp "\\).+").  And indeed,
>
>   (re-search-forward (concat "^\\(?:" diff-hunk-header-re "\\).+"))
>
> fails in diff buffers.  Don't ask me why outline.el adds ".+"
> at the end, I don't know 🤐

That is quite odd, but it's been that way since 2003?  The doc string
says

---
Regular expression to match the beginning of a heading.
Any line whose beginning matches this regexp is considered to start a heading.
---

So it's specifying "the beginning", which could imply that there's
always supposed to be something after it, but it's an odd limitation.

> To circumvent this idiosyncrasy for headings in ‘describe-bindings’, I
> had to use a different regexp in ‘outline-minor-mode-highlight-buffer’
> (not based on font-lock):
>
>   (concat "^\\(?:" outline-regexp "\\).*$")))
>
> where ".+" is replaced with less-strict ".*".

And diff-hunk-header-re definitely specifies an entire line.

Perhaps we could just change the .+ thing to .* and see whether it
breaks anything?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Perhaps we could just change the .+ thing to .* and see whether it
> breaks anything?

Good point.

But am I missing something or commit f0768d3145 didn't change
`diff-hunk-header-re' matching an entire line? I thus don't understand
why Juri is seing a regression w.r. to cycling after that commit has
been installed. Note that I've not been able to reproduce the "working"
behavior described by Juri after reverting that commit.

Juri can you confirm there's a relation between cycling not working and
that commit? Otherwise should we open another bug?
-- 
Matthias

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> Perhaps we could just change the .+ thing to .* and see whether it
>> breaks anything?
>
> Good point.
>
> But am I missing something or commit f0768d3145 didn't change
> `diff-hunk-header-re' matching an entire line? I thus don't understand
> why Juri is seing a regression w.r. to cycling after that commit has
> been installed. Note that I've not been able to reproduce the "working"
> behavior described by Juri after reverting that commit.
>
> Juri can you confirm there's a relation between cycling not working and
> that commit? Otherwise should we open another bug?

I confirm there is a relation because the commit f0768d3145

  +  (when (eq diff-buffer-type 'git)
  +    (setq diff-outline-regexp
  +          (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
  +    (setq-local outline-level #'diff--outline-level))
  +  (setq-local outline-regexp diff-outline-regexp))

changed diff-outline-regexp to contain diff-hunk-header-re that matches an entire line.
This commit replaced the default value of diff-outline-regexp that is

  (defvar diff-outline-regexp
    "\\([*+][*+][*+] [^0-9]\\|@@ ...\\|\\*\\*\\* [0-9].\\|--- [0-9]..\\)")

that didn't match entire lines.

However, changing the .+ thing to .* in outline-font-lock-keywords
fixes this problem.  So we need to decide whether to try to remove
entire line mathing from diff-hunk-header-re (not sure how easy to do),
or just change .+ to .* in outline-font-lock-keywords.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

Juri Linkov <juri@linkov.net> writes:

> I confirm there is a relation because the commit f0768d3145

Oh you're right. Sorry.

> However, changing the .+ thing to .* in outline-font-lock-keywords
> fixes this problem.  So we need to decide whether to try to remove
> entire line mathing from diff-hunk-header-re (not sure how easy to
> do),

I'll have a look.
-- 
Matthias

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

Matthias Meulien <orontee@gmail.com> writes:

> Juri Linkov <juri@linkov.net> writes:
>
>> I confirm there is a relation because the commit f0768d3145
>
> Oh you're right. Sorry.
>
>> However, changing the .+ thing to .* in outline-font-lock-keywords
>> fixes this problem.  So we need to decide whether to try to remove
>> entire line mathing from diff-hunk-header-re (not sure how easy to
>> do),

I just skipped last character of `diff-hunk-header-re' (the culprit `$')
when building `diff-outline-regexp'; Cycling is restored when on hunk
headers. Looks safe to me since `diff-hunk-header-re' is unchanged.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-broken-outline-minor-mode-cycling-in-diff-buffer.patch --]
[-- Type: text/x-diff, Size: 979 bytes --]

From cf4ccd97e6d6e82faee5b01224c5a3caa8908cc1 Mon Sep 17 00:00:00 2001
From: Matthias Meulien <orontee@gmail.com>
Date: Mon, 22 Nov 2021 23:06:06 +0100
Subject: [PATCH] Fix broken outline minor mode cycling in diff buffers

* lisp/vc/diff-mode.el (diff-setup-buffer-type): Fix outline regexp
matching whole line
---
 lisp/vc/diff-mode.el | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 8f83aa580e..eeda503afb 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1588,7 +1588,8 @@ diff-setup-buffer-type
                   nil)))
   (when (eq diff-buffer-type 'git)
     (setq diff-outline-regexp
-          (concat "\\(^diff --git.*\n\\|" diff-hunk-header-re "\\)"))
+          (concat "\\(^diff --git.*\n\\|"
+                  (substring diff-hunk-header-re 0 -1) "\\)"))
     (setq-local outline-level #'diff--outline-level))
   (setq-local outline-regexp diff-outline-regexp))
 
-- 
2.30.2


[-- Attachment #3: Type: text/plain, Size: 14 bytes --]


-- 
Matthias

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Matthias Meulien]

Matthias Meulien <orontee@gmail.com> writes:

>> Juri Linkov <juri@linkov.net> writes:
>>
>>> I confirm there is a relation because the commit f0768d3145
>>
>> Oh you're right. Sorry.
>>
>>> However, changing the .+ thing to .* in outline-font-lock-keywords
>>> fixes this problem.  So we need to decide whether to try to remove
>>> entire line mathing from diff-hunk-header-re (not sure how easy to
>>> do),
>
> I just skipped last character of `diff-hunk-header-re' (the culprit `$')
> when building `diff-outline-regexp'; Cycling is restored when on hunk
> headers. Looks safe to me since `diff-hunk-header-re' is unchanged.

Ok, it works on hunks with "context information" like

   @@ -147,6 +160,61 @@ diff-font-lock-syntax

But it's still broken when there's no "context information" like

   @@ -50,6 +50,7 @@

which argues for your second suggestion, changing
`outline-font-lock-keywords'.
-- 
Matthias

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Matthias Meulien <orontee@gmail.com> writes:

> But it's still broken when there's no "context information" like
>
>    @@ -50,6 +50,7 @@
>
> which argues for your second suggestion, changing
> `outline-font-lock-keywords'.

Yup.  Juri, please go ahead and make the change -- it seems like the
correct fix ere.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> But it's still broken when there's no "context information" like
>>
>>    @@ -50,6 +50,7 @@
>>
>> which argues for your second suggestion, changing
>> `outline-font-lock-keywords'.
>
> Yup.  Juri, please go ahead and make the change -- it seems like the
> correct fix ere.

So now pushed to master.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 780 bytes --]

>>> which argues for your second suggestion, changing
>>> `outline-font-lock-keywords'.
>>
>> Yup.  Juri, please go ahead and make the change -- it seems like the
>> correct fix ere.
>
> So now pushed to master.

This change has such effect that now page delimiters are highlighted
as well.  For an unknown reason, the default value of outline-regexp
includes the page delimiter ^L: "[*^L]+".  So now outline-font-lock-keywords
fontifies them too with the `outline-1' face.  But this highlighting
is not visible, because by default it's overridden by the hardcoded face
`escape-glyph' on ^L.

This begs the question: why outline-regexp includes ‘^L’?
With it the collapsed outlines take twice more space with empty lines,
and without it the outline overview is more compact:


[-- Attachment #2: outline_NEWS.png --]
[-- Type: image/png, Size: 52029 bytes --]

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Juri Linkov <juri@linkov.net> writes:

> This begs the question: why outline-regexp includes ‘^L’?

Yes, I wondered about that, too.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> This begs the question: why outline-regexp includes ‘^L’?
>
> Yes, I wondered about that, too.

I guess no one will notice the absence of ^L because
the default outline-regexp used by everyone in org-mode is
"\\*+ "

diff --git a/lisp/outline.el b/lisp/outline.el
index 5e3d4e0e00..603795cfd3 100644
--- a/lisp/outline.el
+++ b/lisp/outline.el
@@ -42,7 +42,7 @@ outlines
   :prefix "outline-"
   :group 'text)
 
-(defvar outline-regexp "[*\^L]+"
+(defvar outline-regexp "[*]+"
   "Regular expression to match the beginning of a heading.
 Any line whose beginning matches this regexp is considered to start a heading.
 Note that Outline mode only checks this regexp at the start of a line,
-- 

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 16 Dec 2021 19:22:28 +0200
> Cc: 51016@debbugs.gnu.org, Matthias Meulien <orontee@gmail.com>
> 
> >> This begs the question: why outline-regexp includes ‘^L’?
> >
> > Yes, I wondered about that, too.
> 
> I guess no one will notice the absence of ^L because
> the default outline-regexp used by everyone in org-mode is
> "\\*+ "

Why are you talking about Org, when the proposed change is in
outline.el?

Please don't change the behavior of outline.el in
backward-incompatible ways.  This is used for NEWS and similar files,
and those do have ^L separators in them.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> >> This begs the question: why outline-regexp includes ‘^L’?
>> >
>> > Yes, I wondered about that, too.
>>
>> I guess no one will notice the absence of ^L because
>> the default outline-regexp used by everyone in org-mode is
>> "\\*+ "
>
> Why are you talking about Org, when the proposed change is in
> outline.el?

Because most people use Org that has more reasonable defaults.

> Please don't change the behavior of outline.el in
> backward-incompatible ways.  This is used for NEWS and similar files,
> and those do have ^L separators in them.

The proposed change is to improve NEWS and similar files to make
their views more compact by fitting more lines on the screen.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: larsi@gnus.org,  51016@debbugs.gnu.org,  orontee@gmail.com
> Date: Thu, 16 Dec 2021 21:04:37 +0200
> 
> > Why are you talking about Org, when the proposed change is in
> > outline.el?
> 
> Because most people use Org that has more reasonable defaults.

You are free to use Org if that fits your workflows better.  That
doesn't mean we need to ignore users of Outline that don't use Org.

> > Please don't change the behavior of outline.el in
> > backward-incompatible ways.  This is used for NEWS and similar files,
> > and those do have ^L separators in them.
> 
> The proposed change is to improve NEWS and similar files to make
> their views more compact by fitting more lines on the screen.

What does this have to do with removing support for ^L?

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> > Please don't change the behavior of outline.el in
>> > backward-incompatible ways.  This is used for NEWS and similar files,
>> > and those do have ^L separators in them.
>> 
>> The proposed change is to improve NEWS and similar files to make
>> their views more compact by fitting more lines on the screen.
>
> What does this have to do with removing support for ^L?

See this screenshot from one of Juri's previous messages:
(<86lf0l23u1.fsf@mail.linkov.net>)

https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51016;msg=81;att=1;filename=outline_NEWS.png

On the left is NEWS with the default value of outline-regexp, on the
right with ^L taken out.  FWIW I agree that the latter is easier on the
eyes.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>   orontee@gmail.com
> Date: Thu, 16 Dec 2021 22:02:05 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> The proposed change is to improve NEWS and similar files to make
> >> their views more compact by fitting more lines on the screen.
> >
> > What does this have to do with removing support for ^L?
> 
> See this screenshot from one of Juri's previous messages:
> (<86lf0l23u1.fsf@mail.linkov.net>)
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51016;msg=81;att=1;filename=outline_NEWS.png
> 
> On the left is NEWS with the default value of outline-regexp, on the
> right with ^L taken out.  FWIW I agree that the latter is easier on the
> eyes.

And I disagree.  The ^L separators are there for a reason, and even in
the image they do their job.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
>> Cc: Juri Linkov <juri@linkov.net>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>>   orontee@gmail.com
>> Date: Thu, 16 Dec 2021 22:02:05 +0100
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> The proposed change is to improve NEWS and similar files to make
>> >> their views more compact by fitting more lines on the screen.
>> >
>> > What does this have to do with removing support for ^L?
>> 
>> See this screenshot from one of Juri's previous messages:
>> (<86lf0l23u1.fsf@mail.linkov.net>)
>> 
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51016;msg=81;att=1;filename=outline_NEWS.png
>> 
>> On the left is NEWS with the default value of outline-regexp, on the
>> right with ^L taken out.  FWIW I agree that the latter is easier on the
>> eyes.
>
> And I disagree.  The ^L separators are there for a reason, and even in
> the image they do their job.

I'd agree if

(1) Emacs used some display tricks to make the ^L separators look more
    like actual separators (e.g. a full-width horizontal line, as is
    done in C-h o; the page-break-lines package on MELPA tries to make
    FORM FEEDs look a bit like that),

(2) outline-mode treated them as purely visual separators, and not as
    actual section headings reachable with navigation keys.

As things stand, (1) the separators look noisy to me, sort of like stray
control characters, (2) outline-forward-same-level will pause on them
instead of skipping over them and jumping to the next, "actual" heading
(i.e. a heading with actual subsections to show/hide).

It's clear that (1) is purely a visual preference; IMO (2) hints that
outline-regexp conflates characters that are used to define the heading
hierarchy (*) with characters that are used to delimit top-level
sections (^L).

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: 51016@debbugs.gnu.org,  larsi@gnus.org,  orontee@gmail.com,
>   juri@linkov.net
> Date: Fri, 17 Dec 2021 08:07:58 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=51016;msg=81;att=1;filename=outline_NEWS.png
> >> 
> >> On the left is NEWS with the default value of outline-regexp, on the
> >> right with ^L taken out.  FWIW I agree that the latter is easier on the
> >> eyes.
> >
> > And I disagree.  The ^L separators are there for a reason, and even in
> > the image they do their job.
> 
> I'd agree if
> 
> (1) Emacs used some display tricks to make the ^L separators look more
>     like actual separators (e.g. a full-width horizontal line, as is
>     done in C-h o; the page-break-lines package on MELPA tries to make
>     FORM FEEDs look a bit like that),
> 
> (2) outline-mode treated them as purely visual separators, and not as
>     actual section headings reachable with navigation keys.
> 
> As things stand, (1) the separators look noisy to me, sort of like stray
> control characters, (2) outline-forward-same-level will pause on them
> instead of skipping over them and jumping to the next, "actual" heading
> (i.e. a heading with actual subsections to show/hide).
> 
> It's clear that (1) is purely a visual preference; IMO (2) hints that
> outline-regexp conflates characters that are used to define the heading
> hierarchy (*) with characters that are used to delimit top-level
> sections (^L).

I have nothing against enhancing Outline mode to display ^L as we did
in "C-h o", provided that will be an optional feature.  But please
don't argue for _unconditional_ backward-incompatible changes in how
Outline buffers look and behave, for purely subjective aesthetic
reasons, and on top of that for hard-wiring such changes in defvar's.
That is as un-Emacsy as it gets, and frankly I'm surprised something
like that is even being put on the table.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> > And I disagree.  The ^L separators are there for a reason, and even in
>> > the image they do their job.
>> 
>> I'd agree if
>> 
>> (1) Emacs used some display tricks to make the ^L separators look more
>>     like actual separators (e.g. a full-width horizontal line, as is
>>     done in C-h o; the page-break-lines package on MELPA tries to make
>>     FORM FEEDs look a bit like that),
>> 
>> (2) outline-mode treated them as purely visual separators, and not as
>>     actual section headings reachable with navigation keys.
>> 
>> As things stand, (1) the separators look noisy to me, sort of like stray
>> control characters, (2) outline-forward-same-level will pause on them
>> instead of skipping over them and jumping to the next, "actual" heading
>> (i.e. a heading with actual subsections to show/hide).
>> 
>> It's clear that (1) is purely a visual preference; IMO (2) hints that
>> outline-regexp conflates characters that are used to define the heading
>> hierarchy (*) with characters that are used to delimit top-level
>> sections (^L).
>
> I have nothing against enhancing Outline mode to display ^L as we did
> in "C-h o", provided that will be an optional feature.

Right, that covers issue (1); do you have an opinion on issue (2)?
Namely, that ^L being part of outline-regexp causes navigation commands
(e.g. outline-forward-same-level) to treat them the same as top-level
headings?

>                                                         But please
> don't argue for _unconditional_ backward-incompatible changes in how
> Outline buffers look and behave, for purely subjective aesthetic
> reasons, and on top of that for hard-wiring such changes in defvar's.
> That is as un-Emacsy as it gets, and frankly I'm surprised something
> like that is even being put on the table.

FWIW, I don't think issue (2) pertains to aesthetics.  And the behaviour
change I can see (C-c C-f skipping over form feeds) looks like a net
positive.

(Also FWIW, I'm not surprised such radical-sounding changes are
surfacing today; given how often outline-mode has made the headlines
(heh) in NEWS, my impression is that there's been a recent renewal of
interest in this mode which had not seen much love for the past… however
long it's been since it has been introduced[1].

One interpretation is that outline-mode is feature-complete and its
defaults should not be questioned; another interpretation is that
innovation happened elsewhere, and with the benefits of hindsight, some
people are now trying to tune the venerable outline(-minor)-mode to make
it more ergonomic)


[1] Starting from…

$ grep -c '^\*\+ .*outlin' etc/NEWS* | sort -r -k2 -t:

… and checking the results, AFAICT Emacs 28 comes on top, then Emacs 29,
then 20 and 21, and then… basically nothing (except some false positives
in 26 and 24).

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: 51016@debbugs.gnu.org,  larsi@gnus.org,  orontee@gmail.com,
>   juri@linkov.net
> Date: Fri, 17 Dec 2021 22:27:59 +0100
> 
> Right, that covers issue (1); do you have an opinion on issue (2)?

It shouldn't be changed by default and unconditionally, IMO.

> (Also FWIW, I'm not surprised such radical-sounding changes are
> surfacing today; given how often outline-mode has made the headlines
> (heh) in NEWS, my impression is that there's been a recent renewal of
> interest in this mode which had not seen much love for the past… however
> long it's been since it has been introduced[1].

From my POV, there's too much of a tendency to convert Outline to be
Org, and that I cannot support.  People who want to use Org should
just use it; there's no reason to make Outline do the same.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> (Also FWIW, I'm not surprised such radical-sounding changes are
>> surfacing today; given how often outline-mode has made the headlines
>> (heh) in NEWS, my impression is that there's been a recent renewal of
>> interest in this mode which had not seen much love for the past… however
>> long it's been since it has been introduced[1].
>
> From my POV, there's too much of a tendency to convert Outline to be
> Org, and that I cannot support.  People who want to use Org should
> just use it; there's no reason to make Outline do the same.

"Org" is a lot of different things, most of them thoroughly unrelated to
outlining that have no business being integrated into outline-mode, so
thanks for keeping an eye on that tendency.

Personally, as an outline(-minor)-mode user, I'm still very happy that
core Emacs developers are attempting to "sideport" the small set of Org
features that pertains to outlining, e.g. TAB-cycling.  That instantly
enhances all major modes with support for outline-minor-mode, so these
are powerful quality-of-life improvements as far as I am concerned.

Whether these features should be enabled by default is a judgment call,
of course.  As a user, I still consider that Emacs tripping over form
feeds when I ask it to go to the next heading is a plain bug, but I
acknowledge that YMMV 🙃

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

[-- Attachment #1: Type: text/plain, Size: 274 bytes --]

> As a user, I still consider that Emacs tripping over form feeds
> when I ask it to go to the next heading is a plain bug

Indeed, it's a plain bug: when the new variable outline-minor-mode-use-buttons
is enabled, ^L lines displayed as outline headings are making a mess:


[-- Attachment #2: outline-minor-mode-use-buttons.png --]
[-- Type: image/png, Size: 44098 bytes --]

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Cc: Eli Zaretskii <eliz@gnu.org>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>   orontee@gmail.com
> Date: Sat, 18 Dec 2021 19:05:52 +0200
> 
> > As a user, I still consider that Emacs tripping over form feeds
> > when I ask it to go to the next heading is a plain bug
> 
> Indeed, it's a plain bug: when the new variable outline-minor-mode-use-buttons
> is enabled, ^L lines displayed as outline headings are making a mess:

I guess I'm blind, since I see neither a bug nor a mess.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Juri Linkov <juri@linkov.net>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>>   orontee@gmail.com
>> Date: Sat, 18 Dec 2021 19:05:52 +0200
>> 
>> > As a user, I still consider that Emacs tripping over form feeds
>> > when I ask it to go to the next heading is a plain bug
>> 
>> Indeed, it's a plain bug: when the new variable outline-minor-mode-use-buttons
>> is enabled, ^L lines displayed as outline headings are making a mess:
>
> I guess I'm blind, since I see neither a bug nor a mess.

I guess the assumption Juri and I are working with (and maybe Lars too,
since IIUC he added these buttons to act as visual cues to indicate
places which can be expanded or collapsed) is that the purpose of
outline-regexp is to capture "heading" lines, by which we mean titles
and subtitles which define a hierarchy of things to show and hide.

These form feeds do not contribute to the document's hierarchy.  They do
not have subsections to expand or collapse.  Despite this, they are
treated as level-1 headings.

That leads to what Juri and I consider "absurd" results:

- outline-forward-same-level pauses on them: why?  There's nothing for
  a user to expand or collapse there;

- outline-minor-mode-use-buttons adds these clickable buttons: why?
  There's nothing for a user to expand or collapse there.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
> Cc: Juri Linkov <juri@linkov.net>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>   orontee@gmail.com
> Date: Sat, 18 Dec 2021 20:43:18 +0100
> 
> I guess the assumption Juri and I are working with (and maybe Lars too,
> since IIUC he added these buttons to act as visual cues to indicate
> places which can be expanded or collapsed) is that the purpose of
> outline-regexp is to capture "heading" lines, by which we mean titles
> and subtitles which define a hierarchy of things to show and hide.

You are treating NEWS as if it were an Org document.  It isn't.

> These form feeds do not contribute to the document's hierarchy.  They do
> not have subsections to expand or collapse.  Despite this, they are
> treated as level-1 headings.
> 
> That leads to what Juri and I consider "absurd" results:
> 
> - outline-forward-same-level pauses on them: why?  There's nothing for
>   a user to expand or collapse there;
> 
> - outline-minor-mode-use-buttons adds these clickable buttons: why?
>   There's nothing for a user to expand or collapse there.

These are _page_ delimiters.  They are conceptually _above_ level-1
headings.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Kévin Le Gouguec]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Kévin Le Gouguec <kevin.legouguec@gmail.com>
>> Cc: Juri Linkov <juri@linkov.net>,  51016@debbugs.gnu.org,  larsi@gnus.org,
>>   orontee@gmail.com
>> Date: Sat, 18 Dec 2021 20:43:18 +0100
>> 
>> I guess the assumption Juri and I are working with (and maybe Lars too,
>> since IIUC he added these buttons to act as visual cues to indicate
>> places which can be expanded or collapsed) is that the purpose of
>> outline-regexp is to capture "heading" lines, by which we mean titles
>> and subtitles which define a hierarchy of things to show and hide.
>
> You are treating NEWS as if it were an Org document.  It isn't.

I don't see where Org enters the picture from my description?  AFAICT
this "assumption" I described is just paraphrasing outline-mode's
docstring:

> Set major mode for editing outlines with selective display.
> Headings are lines which start with asterisks: one for major headings,
> two for subheadings, etc.  Lines not starting with asterisks are body lines.
> 
> Body text or subheadings under a heading can be made temporarily
> invisible, or visible again.

>> These form feeds do not contribute to the document's hierarchy.  They do
>> not have subsections to expand or collapse.  Despite this, they are
>> treated as level-1 headings.
>> 
>> That leads to what Juri and I consider "absurd" results:
>> 
>> - outline-forward-same-level pauses on them: why?  There's nothing for
>>   a user to expand or collapse there;
>> 
>> - outline-minor-mode-use-buttons adds these clickable buttons: why?
>>   There's nothing for a user to expand or collapse there.
>
> These are _page_ delimiters.  They are conceptually _above_ level-1
> headings.

Right; unfortunately, in addition to this *conceptual* understanding of
form feeds, *functionally* they are treated as level-1 headings, with
the unfortunate consequences we highlighted.

As you point out, they are page delimiters; if we were to take them out
of outline-regexp, page commands (navigation, marking, narrowing) would
keep working just fine!

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Lars Ingebrigtsen]

Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:

> Right; unfortunately, in addition to this *conceptual* understanding of
> form feeds, *functionally* they are treated as level-1 headings, with
> the unfortunate consequences we highlighted.
>
> As you point out, they are page delimiters; if we were to take them out
> of outline-regexp, page commands (navigation, marking, narrowing) would
> keep working just fine!

Yes, I don't really see any reason to keep ^L in the outline regexp,
because people that switch outline mode on wants that to be the "top
level" organisation, while people that don't do that, will still have
page-wise navigation with ^L.  (And as you point out, the page-wise
commands still work fine even if you don't display the ^L's.)

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  51016@debbugs.gnu.org,
>   orontee@gmail.com,  juri@linkov.net
> Date: Sun, 19 Dec 2021 12:00:14 +0100
> 
> Kévin Le Gouguec <kevin.legouguec@gmail.com> writes:
> 
> > Right; unfortunately, in addition to this *conceptual* understanding of
> > form feeds, *functionally* they are treated as level-1 headings, with
> > the unfortunate consequences we highlighted.
> >
> > As you point out, they are page delimiters; if we were to take them out
> > of outline-regexp, page commands (navigation, marking, narrowing) would
> > keep working just fine!
> 
> Yes, I don't really see any reason to keep ^L in the outline regexp,

I do, so please don't make that change unconditionally.

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > > This begs the question: why outline-regexp includes ‘^L’?

  > Yes, I wondered about that, too.

^L at the beginning of a line separates pages.
I don't know exactly what that regexp is for, but that is
probably why ^L appears.

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#51016: 28.0.50; 'diff-font-lock-prettify' breaks display of outline headers

[Juri Linkov]

>> To circumvent this idiosyncrasy for headings in ‘describe-bindings’, I
>> had to use a different regexp in ‘outline-minor-mode-highlight-buffer’
>> (not based on font-lock):
>>
>>   (concat "^\\(?:" outline-regexp "\\).*$")))
>>
>> where ".+" is replaced with less-strict ".*".
>
> And diff-hunk-header-re definitely specifies an entire line.
>
> Perhaps we could just change the .+ thing to .* and see whether it
> breaks anything?

This looks like the right thing to do.  And this regexp in outline-mode

  (setq-local imenu-generic-expression
	      (list (list nil (concat "^\\(?:" outline-regexp "\\).*$") 0)))

also uses .* instead of .+, probably because .+ caused a problem in imenu.