Re: [PATCH] ox-icalendar.el: customizable vevent summary prefix

[Ihor Radchenko <yantar92@gmail.com>]

Mikhail Skorzhisnkii <mskorzhinskii@eml.cc> writes:

> I have signed FSF papers. Attaching a rebased patch with additional changes to
> ORG-NEWS

Thanks!

> Subject: [PATCH 1/2] org-agenda.el: customize outline path in echo area
>
> * lisp/org-agenda.el (org-agenda-show-outline-path): add an option to
> show document title in outline path (instead of file name)

Please follow the commit message conventions as described in
https://orgmode.org/worg/org-contribute.html#commit-messages In
particular, start sentences from capital letters, end them with ".",
separate sentences with double space, and quote lisp symbols as
`symbol'.

> * lisp/org.el (org-get-title-from-buffer): a function to collect the
                                             New
> document title from the org-mode buffer
                                         .


> * lisp/org.el (org-display-outline-path): add logic that will collect a
> document title and put it into the outline path if
> org-agenda-show-outline-path set to 'title

This is not what the patch does.  From this message, it looks like
`org-agenda-show-outline-path' is affecting the output of
`org-display-outline-path', which is not true.

>  (defcustom org-agenda-show-outline-path t
> -  "Non-nil means show outline path in echo area after line motion."
> +  "Non-nil means show outline path in echo area after line motion.
> +
> +If set to 'title, show document title."

This is not very clear. I'd rather put more detailed explanation as in
the defcustom :type spec below.

>    :group 'org-agenda-startup
> -  :type 'boolean)
> +  :type '(choice
> +	  (const :tag "Don't show outline path in agenda view." nil)
> +	  (const :tag "Show outline path with prepended file name." t)
> +	  (const :tag "Show outline path with prepended document title. Fallback to file name is no title is present." title)))
> -(defun org-display-outline-path (&optional file current separator just-return-string)
> +(defun org-get-title-from-buffer (&optional buffer)
> +  "Collect title from the provided `org-mode' BUFFER."
> +  (let* ((buffer (or buffer (current-buffer)))
> +         (buffer (or (buffer-base-buffer buffer)
> +                     buffer))

Why not just

(or (buffer-base-buffer buffer)
    buffer
    (current-buffer))

> +         title)
> +    (with-current-buffer buffer
> +      (pcase (org-collect-keywords '("TITLE"))
> +        (`(("TITLE" . ,val))
> +         (setq title (car val)))))
> +    title))

Extra `title' variable is unnecessary here. You can simply do 

    (with-current-buffer buffer
      (pcase (org-collect-keywords '("TITLE"))
        (`(("TITLE" ,val . _))
         val)))

Also, what will happen in a file like

#+TITLE: Begin title
#+TITLE: .. end title

?

> +(defun org-display-outline-path (&optional file-or-title current separator just-return-string)
>    "Display the current outline path in the echo area.
>  
> -If FILE is non-nil, prepend the output with the file name.
> +If FILE-OR-TITLE is 'title, prepend outline with file title.  If
> +it is non-nil or title is not present in document, prepend
> +outline path with the file name.
>  If CURRENT is non-nil, append the current heading to the output.
>  SEPARATOR is passed through to `org-format-outline-path'.  It separates
>  the different parts of the path and defaults to \"/\".
> @@ -7407,6 +7421,8 @@ If JUST-RETURN-STRING is non-nil, return a string, don't display a message."
>    (interactive "P")
>    (let* (case-fold-search
>  	 (bfn (buffer-file-name (buffer-base-buffer)))
> +         (title-prop (when (and file-or-title (eq file-or-title 'title))

can be simply (eq file-or-title 'title)

-- 
Ihor Radchenko,
Org mode contributor,
Learn more about Org mode at https://orgmode.org/.
Support Org development at https://liberapay.com/org-mode,
or support my work at https://liberapay.com/yantar92