Headline colors not diplayed correctly

kiwanami / emacs-calfw

A calendar framework for Emacs

1.16k stars 100 forks source link

Headline colors not diplayed correctly #152

Closed kafu16 closed 1 year ago

kafu16 commented 1 year ago

At first, thank you very much for this great package!

Some headlines are not displayed correctly as shown in the screenshot below. In some cases the colors are displayed in green (this color can be changed using the variable cfw:org-face-agenda-item-foreground-color) plus additional horizontal blue lines (see 12th, 17th, 18th). I would like the headlines to be displayed in red and the todo-items with the colors defined for them (see 19th).

calfw_issue_colors_arrows

I did not find out when exactly this happens. It seems that the headlines are displayed correctly when no specific time is set the and when headlines lead to line breaks. Short headlines are sometimes displayed correctly, sometimes not.

I use these costom colors for the todo-items:

(setq org-todo-keyword-faces
      '(("NEXT" . org-warning) ("TODO" . org-warning) ("WAITING" . org-warning) ("TERMIN" . "deep sky blue")
    ("SOMEDAY" . "brown") ("PROJ" . "magenta") ("DONE" . "DarkGreen") ("CANCELLED" . "DarkGreen")))

And I am using this fix for the time ranges. However, the described behaviour still appears when I do not use the fix for the time ranges.

I would really appreciate any help :).

For reproducing: These are the headlines I used

* Time range
  <2023-01-05 Do>--<2023-01-14 Sa>

* TERMIN No time specified
  DEADLINE: <2023-01-12 Do>

* TERMIN Time specified
  DEADLINE: <2023-01-12 Do 12:00>

* TERMIN Time specified, with a very long headline
  DEADLINE: <2023-01-12 Do 12:00>

* DONE not displayed correctly
  CLOSED: [2023-01-08 So 17:47] DEADLINE: <2023-01-17 Di 12:00>

* TODO Short todo, correct
   DEADLINE: <2023-01-17 Di 21:00>

* PROJ No time specified
  DEADLINE: <2023-01-18 Mi>

* PROJ Time specified
  DEADLINE: <2023-01-18 Mi 12:00>

* PROJ Time specified, with a very long headline
  DEADLINE: <2023-01-18 Mi 12:00>

* TODO And 
  DEADLINE: <2023-01-19 Do 10:00>

* PROJ then everything 
  DEADLINE: <2023-01-19 Do 15:00>

* TERMIN is 
  DEADLINE: <2023-01-19 Do 17:00>

* DONE displayed correctly
  CLOSED: [2023-01-08 So 17:41] DEADLINE: <2023-01-19 Do 19:00>  

* TODO again  
  DEADLINE: <2023-01-19 Do 21:00>  

* TODO all short headlines. 
  DEADLINE: <2023-01-19 Do 21:00>

lawlist commented 1 year ago

cfw:render-separator is examining the beginning of the string to determine what face to apply to the entire string. The org-agenda data does not come out-of-the-box with the time pre-pended. Keep in mind that the data is from the org-agenda mechanism that uses text-properties, and the faces are different than the flat org-mode buffers use; e.g., org-agenda-done is one of those faces. FYI: The entire entry is not being word-wrapped per se, but is instead being broken into parts such that each line has its own string.

Consider changing:

OLD:

(let ((last-face (get-text-property 0 'face string)))
...

NEW:

(let ((last-face
        (if (string-match "[0-9]+:[0-9]+\s" string)
          (get-text-property (match-end 0) 'face string)
          (get-text-property 0 'face string))))
...

kafu16 commented 1 year ago

Thank you very much, this is very nice of you!

The result of your code snippet is the following calfw_issue_colors_arrows_first_fix

On the 17th and 18th the colors of the first lines have changed but they are still not displayed as they should be. I would like the time to be displayed in green, the colors of the TODO-items as they are defined in my init.el and the rest of the headline unicolor in red as on the 12th and the 19th. Here, everything is displayed correctly (as on the screenshot in the initial post). I haven't got the time to dig deeper into your explanations but I'm also not sure if I would be able to fix it myself as I'm not familiar with Emacs Lisp.

lawlist commented 1 year ago

Here is a second draft: This is still a work in progress, and it still needs code to deal with a situation when org-todo-keywords-for-agenda is not defined (but that is the easy part to fix). FYI: The initial settings are missing org-todo-keywords definition, and for testing purposes I went ahead and set org-todo-keywords-for-agenda rather than opening the agenda buffer each time.

(defun cfw:render-separator (string)
  "[internal] Add a separator into the ROWS list."
  (when (get-text-property 0 'cfw:item-separator string)
    (let* ((face-fn
             (lambda (beg end the-face)
               (cond
                 ((or (null the-face) (listp the-face))
                    (setq the-face (append the-face `(:underline ,cfw:face-item-separator-color)))
                    (put-text-property beg end 'face the-face string)
                    (put-text-property beg end 'font-lock-face the-face string))
                ((symbolp the-face)
                   (let ((attrs (face-all-attributes the-face (selected-frame))))
                     (setq attrs ; transform alist to plist
                           (loop with nattrs = nil
                                 for (n . v) in (append attrs `((:underline . ,cfw:face-item-separator-color)))
                                 do (setq nattrs (cons n (cons v nattrs)))
                                 finally return nattrs))
                     (put-text-property beg end 'face attrs string)
                     (put-text-property beg end 'font-lock-face attrs string)))
                (t
                  (message "DEBUG? CFW: FACE %S / %S" string the-face)))))
           (re (if (not (null org-todo-keywords-for-agenda))
                 (concat "^\\([0-9]+:[0-9]+\\)?"
                         "\\([\s]\\)?"
                         "\\(" (mapconcat 'identity org-todo-keywords-for-agenda "\\|") "\\)?"
                         "\\([\s]\\)?"
                         "\\(.*\\)$")
                 "^.*$"))
           (time-face (and (string-match re string)
                           (match-beginning 1)
                           (get-text-property (match-beginning 1) 'face string)))
           (time-string (and (string-match re string)
                             (match-beginning 1)
                             (match-string 1 string)
                             (funcall face-fn (match-beginning 1) (match-end 1) time-face)))
           (first-space-string (and (string-match re string)
                                    (match-beginning 2)
                                    (match-string 2 string)
                                    (funcall face-fn (match-beginning 2) (match-end 2) nil)))
           (keyword-face (and (string-match re string)
                              (match-beginning 3)
                              (get-text-property (match-beginning 3) 'face string)))
           (keyword-string (and (string-match re string)
                                (match-beginning 3)
                                (match-string 3 string)
                                (funcall face-fn (match-beginning 3) (match-end 3) keyword-face)))
           (second-space-string (and (string-match re string)
                                     (match-beginning 4)
                                     (match-string 4 string)
                                     (funcall face-fn (match-beginning 4) (match-end 4) nil)))
           (title-face (and (string-match re string)
                            (match-beginning 5)
                            (get-text-property (match-beginning 5) 'face string)))
           (title-string (and (string-match re string)
                              (match-beginning 5)
                              (match-string 5 string)
                              (funcall face-fn (match-beginning 5) (match-end 5) title-face))))))
  string)

kafu16 commented 1 year ago

Wow, it looks very good now!

calfw_issue_colors_second_fix

The only thing is that there are still these horizontal lines with the entries whose colors were not displayed correctly before this fix?

lawlist commented 1 year ago

cfw:render-break-lines: The horizontal underlines on the 17th and 18th are being placed programmatically based upon the combined length of the total entries for each date. I.e., the algorithm determines that there is more room on the 17th and the 18th, verses the opposite is true for the 12th and the 19th. The 12th analysis is skewed because the time range hyphens spanning the cell are counted as part of the total combined length of all entries in that cell.

The time-range feature is triggered by the first entry:

* Time range
  <2023-01-05 Do>--<2023-01-14 Sa>

lawlist commented 1 year ago

Third (3rd) draft, which now deals with the situation where org-todo-keywords-for-agenda is not yet defined (e.g., because the user did not first create an agenda buffer).

(defun cfw:render-separator (string)
  "[internal] Add a separator into the ROWS list."
  (when (get-text-property 0 'cfw:item-separator string)
    (let ((face-fn
            (lambda (beg end the-face)
              (cond
                ((or (null the-face) (listp the-face))
                   (setq the-face (append the-face `(:underline ,cfw:face-item-separator-color)))
                   (put-text-property beg end 'face the-face string)
                   (put-text-property beg end 'font-lock-face the-face string))
                ((symbolp the-face)
                   (let ((attrs (face-all-attributes the-face (selected-frame))))
                     (setq attrs ; transform alist to plist
                           (loop with nattrs = nil
                                 for (n . v) in (append attrs `((:underline . ,cfw:face-item-separator-color)))
                                 do (setq nattrs (cons n (cons v nattrs)))
                                 finally return nattrs))
                     (put-text-property beg end 'face attrs string)
                     (put-text-property beg end 'font-lock-face attrs string)))
                (t
                  (message "DEBUG? CFW: FACE %S / %S" string the-face))))))
      (if (null org-todo-keywords-for-agenda)
        (funcall face-fn 0 (length string) (get-text-property 0 'face string))
        (let* ((re (concat
                     "^\\([0-9]+:[0-9]+\\)?"
                     "\\([\s]\\)?"
                     "\\(" (mapconcat 'identity org-todo-keywords-for-agenda "\\|") "\\)?"
                     "\\([\s]\\)?"
                     "\\(.*\\)$"))
               (time-face (and (string-match re string)
                               (match-beginning 1)
                               (get-text-property (match-beginning 1) 'face string)))
               (time-string (and (string-match re string)
                                 (match-beginning 1)
                                 (match-string 1 string)
                                 (funcall face-fn (match-beginning 1) (match-end 1) time-face)))
               (first-space-string (and (string-match re string)
                                        (match-beginning 2)
                                        (match-string 2 string)
                                        (funcall face-fn (match-beginning 2) (match-end 2) nil)))
               (keyword-face (and (string-match re string)
                                  (match-beginning 3)
                                  (get-text-property (match-beginning 3) 'face string)))
               (keyword-string (and (string-match re string)
                                    (match-beginning 3)
                                    (match-string 3 string)
                                    (funcall face-fn (match-beginning 3) (match-end 3) keyword-face)))
               (second-space-string (and (string-match re string)
                                         (match-beginning 4)
                                         (match-string 4 string)
                                         (funcall face-fn (match-beginning 4) (match-end 4) nil)))
               (title-face (and (string-match re string)
                                (match-beginning 5)
                                (get-text-property (match-beginning 5) 'face string)))
               (title-string (and (string-match re string)
                                  (match-beginning 5)
                                  (match-string 5 string)
                                  (funcall face-fn (match-beginning 5) (match-end 5) title-face))))))))
  string)

lawlist commented 1 year ago

For future reference, here are my init.el settings used for working on this particular issue:

(setq debug-on-error t)

(add-to-list 'load-path "/home/lawlist/.emacs.d/emacs-calfw/")

(setq inhibit-startup-message t
      org-todo-keywords '((sequence "NEXT" "TODO" "WAITING" "TERMIN" "SOMEDAY" "PROJ"
                           "|" "DONE" "CANCELLED"))
      org-todo-keyword-faces
        '(("NEXT" . org-warning)
          ("TODO" . org-warning)
          ("WAITING" . org-warning)
          ("TERMIN" . "deep sky blue")
          ("SOMEDAY" . "brown")
          ("PROJ" . "magenta")
          ("DONE" . "DarkGreen")
          ("CANCELLED" . "DarkGreen"))
       org-todo-keywords-for-agenda
         '("NEXT" "TODO" "WAITING" "TERMIN" "SOMEDAY" "PROJ" "DONE" "CANCELLED"))

(setq org-agenda-files '("/home/lawlist/.emacs.d/test.org"))

(require 'calfw-org)

(toggle-frame-maximized)

(add-hook 'emacs-startup-hook 'cfw:open-org-calendar)

kafu16 commented 1 year ago

So I tested your 3rd draft in comparison to your 2nd draft (also with your given init.el): With the 3rd draft and not defining org-todo-keywords-for-agenda in init.el I still have to create the agenda buffer before I call M-x cfw:open-org-calendar in order to have the colors displayed correctly. But I think defining org-todo-keywords-for-agenda once in the init.el is not a problem and I wouldn't consider this an issue.

kafu16 commented 1 year ago

cfw:render-break-lines: The horizontal underlines on the 17th and 18th are being placed programmatically based upon the combined length of the total entries for each date. I.e., the algorithm determines that there is more room on the 17th and the 18th, verses the opposite is true for the 12th and the 19th. The 12th analysis is skewed because the time range hyphens spanning the cell are counted as part of the total combined length of all entries in that cell.

Thank you for the explanation! Is there a way to get rid of the horizontal lines? I tried to adapt the line breaking (see https://github.com/kiwanami/emacs-calfw#line-breaking) but this did not help.

lawlist commented 1 year ago

Thank you for the explanation! Is there a way to get rid of the horizontal lines? I tried to adapt the line breaking (see https://github.com/kiwanami/emacs-calfw#line-breaking) but this did not help.

Within cfw:render-break-lines, there is no built-in customization to disable the item line division feature. Commenting out (when total-rows (cfw:render-add-item-separator-sign total-rows)) would achieve the desired effect.

lawlist commented 1 year ago

... But I think defining org-todo-keywords-for-agenda once in the init.el is not a problem and I wouldn't consider this an issue.

Agreed. The function cfw:open-org-calendar contains a warning reminder for the user to first create the *Org Agenda* buffer when org-todo-keywords-for-agenda has not yet been defined: (when (not org-todo-keywords-for-agenda) (message "Warn : open org-agenda buffer first.")) When testing, I was too lazy to create the agenda buffer and wanted to kill and restart Emacs as quickly as possible to debug ... I used a different instance of Emacs to make changes to the code.

kafu16 commented 1 year ago

Within cfw:render-break-lines, there is no built-in customization to disable the item line division feature. Commenting out (when total-rows (cfw:render-add-item-separator-sign total-rows)) would achieve the desired effect.

Great, this works, thank you very much! It looks very nice now :).