bug#13072: 24.3.50; Fancy Diary display fontification failures

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Stephen Berman]

In GNU Emacs 24.3.50.6 (x86_64-suse-linux-gnu, GTK+ Version 3.4.4)
 of 2012-12-03 on rosalinde
Bzr revision: 111073 dmantipov@yandex.ru-20121203080602-hwv4fug7bvt2red7
Windowing system distributor `The X.Org Foundation', version 11.0.11203000
System Description:	openSUSE 12.2 (x86_64)

Configured using:
 `configure '--without-toolkit-scroll-bars' 'CFLAGS=-g3 -O0''

Important settings:
  value of $LANG: en_US.UTF-8
  value of $XMODIFIERS: @im=local
  locale-coding-system: utf-8-unix
  default enable-multibyte-characters: t


The doc string of calendar-date-display-form says:

    For example, a typical American form would be
    '(month "/" day "/" (substring year -2))
    whereas
    '((format "%9s, %9s %2s, %4s" dayname monthname day year))
    would give the usual American style in fixed-length fields.

But if you set calendar-date-display-form to either of these values
(either via setq in your user-init-file or via the Custom interface),
then in the Fancy Diary display the date header line is not fontified.
To reproduce:

0. Let the file ~/diary consist of entries dated today, like these:
+------------------
| Dec 3, 2012 test1
| 12/3/2012 test2
| 12/3/12 test3
+------------------

1. emacs -Q
2. M-x calendar
3. In the *Calendar* buffer type `d' to get the Fancy Diary display, and
note the fontified date header:
+------------------
| Monday, December 3, 2012
| ========================
| test2
| test3
| test1
+------------------

4. Kill the *Fancy Diary Entries* buffer.
5. Type `M-x customize-option RET calendar-date-display-form RET',
change the value from the default `((if dayname (concat dayname ", "))
monthname " " day ", " year)' to `(month "/" day "/" (substring year
-2))', then repeat steps 2 and 3.
=> The date header in the Fancy Diary display is not fontified.

Now repeat steps 4 and 5, this time changing the value to `((format
"%9s, %9s %2s, %4s" dayname monthname day year))', then repeat steps 2
and 3 again.
=> Again, the date header in the Fancy Diary display is not fontified.

If you restore the default value and repeat steps 2 and 3, now the date
header is fontified again.  (Note, too, the all the entries in the
~/diary have correctly fontified headers, regardless of the value of
calendar-date-display-form.)

I think these two cases (using `substring' and using `format' in the
value of calendar-date-display-form) fail to fontify for different
reasons:

- In the substring case, the problem is that in
  diary-fancy-date-pattern, the form `(substring year -2)' is evaluated
  with `year' let-bound to "3", which raises an args-out-of-range error,
  short-circuiting fontification.

- In contrast, the `format' form returns a string with the variables
  replaced by their let-bound values, which evaluates to itself and is
  passed to replace-regexp-in-string, so diary-fancy-date-pattern
  returns a valid regexp for the Fancy Diary header, so fontification
  should occur AFAICT, yet it doesn't.  However, this appears to depend
  on the pattern and values of the width specifiers in the format
  string, in a way I don't understand.  From testing lots of
  combinations, I found the following generalization appears to hold: if
  the second `%s' sequence (corresponding to `monthname') has a width
  specifier not greater than 8 (note that `monthname' evaluates to
  "December", a string of 9 characters; I haven't tested other month
  names yet) and the fourth `%s' sequence (corresponding to `year') has
  a width specifier not greater than 1, then in the Fancy Diary display,
  the date header is correctly fontified, regardless of the values (or
  presence) of the first and third width specifiers (corresponding to
  `dayname' and `day'); otherwise, fontification fails.  (I haven't yet
  tested different numbers of `%s' sequences.)

I have no idea what's going on with the `format' cases; but if an
explanation can be found for the behavior I described, which is
independent of how diary-fancy-date-pattern builds the regexp -- in
other words, if we assume that the regexp resulting from `format' in
general is valid for fontification, then the following patch, which
fixes the fontification failure in the `substring' case (and generalizes
it to all non-atomic sexps aside from `format' sexps) is one possible
half of the solution.


=== modified file 'lisp/calendar/diary-lib.el'
*** lisp/calendar/diary-lib.el	2012-11-27 15:40:49 +0000
--- lisp/calendar/diary-lib.el	2012-12-03 23:48:18 +0000
***************
*** 2461,2469 ****
       ;; string form"; eg the iso version calls string-to-number on some.
       ;; Therefore we cannot eg just let day = "[0-9]+".  (Bug#8583).
       ;; Assumes no integers in c-day/month-name-array.
!      (replace-regexp-in-string "[0-9]+" "[0-9]+"
!                                (mapconcat 'eval calendar-date-display-form "")
!                                nil t))
     ;; Optional ": holiday name" after the date.
     "\\(: .*\\)?"))
  
--- 2461,2478 ----
       ;; string form"; eg the iso version calls string-to-number on some.
       ;; Therefore we cannot eg just let day = "[0-9]+".  (Bug#8583).
       ;; Assumes no integers in c-day/month-name-array.
!      (replace-regexp-in-string
!       "[0-9]+" "[0-9]+"
!       (mapconcat (lambda (x) (if (or (atom x) (eq (car x) 'format))
! 				 (eval x)
! 			       (catch 'end
! 				 (while (consp x)
! 				   (let ((y (pop x)))
! 				     (when (or (eq y 'year) (eq y 'month)
! 					       (eq y 'day))
! 				       (throw 'end (eval y))))))))
! 		 calendar-date-display-form "")
!       nil t))
     ;; Optional ": holiday name" after the date.
     "\\(: .*\\)?"))
  

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> The doc string of calendar-date-display-form says:
>
>     For example, a typical American form would be
>     '(month "/" day "/" (substring year -2))
>     whereas
>     '((format "%9s, %9s %2s, %4s" dayname monthname day year))
>     would give the usual American style in fixed-length fields.
>
> But if you set calendar-date-display-form to either of these values
> (either via setq in your user-init-file or via the Custom interface),
> then in the Fancy Diary display the date header line is not fontified.

I've respun your patch for Emacs 28...

However, I'm not really sure why Fancy Diary Mode is doing any of
this -- it seems kinda misguided, and as you say, your patch only fixes
half of the problem.

Instead of inserting the heading, and then trying to make a regexp to
match the heading and letting font-lock fontize, why doesn't it just put
the correct face on the heading when it inserts it?  Because trying to
make that regexp is bound to have strange edge cases.

Historical reasons?

diff --git a/lisp/calendar/diary-lib.el b/lisp/calendar/diary-lib.el
index da98e44926..8ea831554b 100644
--- a/lisp/calendar/diary-lib.el
+++ b/lisp/calendar/diary-lib.el
@@ -2406,9 +2406,18 @@ diary-fancy-date-pattern
      ;; string form"; eg the iso version calls string-to-number on some.
      ;; Therefore we cannot eg just let day = "[0-9]+".  (Bug#8583).
      ;; Assumes no integers in c-day/month-name-array.
-     (replace-regexp-in-string "[0-9]+" "[0-9]+"
-                               (mapconcat #'eval calendar-date-display-form "")
-                               nil t))
+      (replace-regexp-in-string
+       "[0-9]+" "[0-9]+"
+       (mapconcat (lambda (x)
+                    (if (or (atom x) (eq (car x) 'format))
+ 			(eval x)
+ 		      (catch 'end
+ 			(while (consp x)
+ 			  (let ((y (pop x)))
+ 			    (when (or (eq y 'year) (eq y 'month) (eq y 'day))
+ 			      (throw 'end (eval y))))))))
+ 		  calendar-date-display-form "")
+       nil t))
    ;; Optional ": holiday name" after the date.
    "\\(: .*\\)?"))
 


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

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Stephen Berman]

On Fri, 21 Aug 2020 14:35:08 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> The doc string of calendar-date-display-form says:
>>
>>     For example, a typical American form would be
>>     '(month "/" day "/" (substring year -2))
>>     whereas
>>     '((format "%9s, %9s %2s, %4s" dayname monthname day year))
>>     would give the usual American style in fixed-length fields.
>>
>> But if you set calendar-date-display-form to either of these values
>> (either via setq in your user-init-file or via the Custom interface),
>> then in the Fancy Diary display the date header line is not fontified.
>
> I've respun your patch for Emacs 28...
>
> However, I'm not really sure why Fancy Diary Mode is doing any of
> this -- it seems kinda misguided, and as you say, your patch only fixes
> half of the problem.
>
> Instead of inserting the heading, and then trying to make a regexp to
> match the heading and letting font-lock fontize, why doesn't it just put
> the correct face on the heading when it inserts it?  Because trying to
> make that regexp is bound to have strange edge cases.
>
> Historical reasons?

I don't know, but a first rough implementation of your suggestion seems
promising: with the patch below, when using either of the above values
of calendar-date-display-form, the date strings are fontified.  A
complete fix should go through the code carefully, checking for possible
corner cases.  Glenn Morris probably knows best if such an approach is
worth pursuing.

Steve Berman

diff --git a/lisp/calendar/diary-lib.el b/lisp/calendar/diary-lib.el
index da98e44926..2d024c04d9 100644
--- a/lisp/calendar/diary-lib.el
+++ b/lisp/calendar/diary-lib.el
@@ -1092,7 +1092,8 @@ diary-fancy-display
                 (if (calendar-date-equal date (car h))
                     (setq date-holiday-list (append date-holiday-list
                                                     (cdr h)))))
-              (insert (if (bobp) "" ?\n) (calendar-date-string date))
+              (insert (if (bobp) "" ?\n)
+                      (propertize (calendar-date-string date) 'face 'diary))
               (if date-holiday-list (insert ":  "))
               (setq cc (current-column))
               (insert (mapconcat (lambda (x)
@@ -2460,11 +2461,6 @@ diary-fancy-overriding-map
 (define-derived-mode diary-fancy-display-mode special-mode
   "Diary"
   "Major mode used while displaying diary entries using Fancy Display."
-  (set (make-local-variable 'font-lock-defaults)
-       '(diary-fancy-font-lock-keywords
-         t nil nil nil
-         (font-lock-fontify-region-function
-          . diary-fancy-font-lock-fontify-region-function)))
   (set (make-local-variable 'minor-mode-overriding-map-alist)
        (list (cons t diary-fancy-overriding-map)))
   (view-mode 1))

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> I don't know, but a first rough implementation of your suggestion seems
> promising: with the patch below, when using either of the above values
> of calendar-date-display-form, the date strings are fontified.  A
> complete fix should go through the code carefully, checking for possible
> corner cases.  Glenn Morris probably knows best if such an approach is
> worth pursuing.

[...]

> -              (insert (if (bobp) "" ?\n) (calendar-date-string date))
> +              (insert (if (bobp) "" ?\n)
> +                      (propertize (calendar-date-string date) 'face 'diary))

Makes sense to me.  And we could then probably remove
diary-fancy-date-pattern.

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

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Stephen Berman]

On Sat, 22 Aug 2020 15:26:38 +0200 Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Stephen Berman <stephen.berman@gmx.net> writes:
>
>> I don't know, but a first rough implementation of your suggestion seems
>> promising: with the patch below, when using either of the above values
>> of calendar-date-display-form, the date strings are fontified.  A
>> complete fix should go through the code carefully, checking for possible
>> corner cases.  Glenn Morris probably knows best if such an approach is
>> worth pursuing.
>
> [...]
>
>> -              (insert (if (bobp) "" ?\n) (calendar-date-string date))
>> +              (insert (if (bobp) "" ?\n)
>> +                      (propertize (calendar-date-string date) 'face 'diary))
>
> Makes sense to me.  And we could then probably remove
> diary-fancy-date-pattern.

That sounds like a good simplification of the code, but I think Glenn
should chime in before any such change is made.

Steve Berman

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

> That sounds like a good simplification of the code, but I think Glenn
> should chime in before any such change is made.

Sure (but Glenn removed himself as the maintainer last year, in case you
didn't know).

diff --git a/lisp/calendar/diary-lib.el b/lisp/calendar/diary-lib.el
index 1be2a05eee..369a93ca17 100644
--- a/lisp/calendar/diary-lib.el
+++ b/lisp/calendar/diary-lib.el
@@ -4,7 +4,7 @@
 ;; Foundation, Inc.
 
 ;; Author: Edward M. Reingold <reingold@cs.uiuc.edu>
-;; Maintainer: Glenn Morris <rgm@gnu.org>
+;; Maintainer: emacs-devel@gnu.org
 ;; Keywords: calendar
 
 ;; This file is part of GNU Emacs.


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

bug#13072: 24.3.50; Fancy Diary display fontification failures

[Lars Ingebrigtsen]

Stephen Berman <stephen.berman@gmx.net> writes:

>> Makes sense to me.  And we could then probably remove
>> diary-fancy-date-pattern.
>
> That sounds like a good simplification of the code, but I think Glenn
> should chime in before any such change is made.

I've now applied a somewhat reworked version of your patch to the
trunk.  I'm keeping the fancy font lock stuff, but just eliding the
date header match.

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