Hi, Ihor, Sorry for the delay with fixes, took some time before I got time to finish this. Thanks for your review and looking forward for the next iteration. See new version in the attachment. Comments are inline. Ihor Radchenko writes: > Mikhail Skorzhisnkii writes: > >> Thank you for suggestion, I seen an announcement about this function, but somehow forgot about it. >> >> Sending next version of these patches. Changes from the next version: > > Thanks! > >> Subject: [PATCH 3/3] org-refile.el: show refile targets with a title >> >> * lisp/org-refile.el (org-refile-get-targets): Use a document >> title (#+TITLE) instead of file or buffer name in outline path, if >> a corresponding customisation option is set to ’title. Fallback to a >> filename if there is no title in the document. > > Please use 2 spaces between sentences in docstrings, comments, and > commit messages. Also, end sentences with “.”. See > and > > Fixed. >> (defcustom org-outline-path-complete-in-steps t >> “Non-nil means complete the outline path in hierarchical steps. >> @@ -319,6 +320,11 @@ converted to a headline before refiling.” >> (push (list (and (buffer-file-name (buffer-base-buffer)) >> (file-truename (buffer-file-name (buffer-base-buffer)))) >> f nil nil) tgs)) >> + (when (eq org-refile-use-outline-path ’title) >> + (push (list (or (org-get-title) >> + (and f (file-name-nondirectory f))) >> + f nil nil) >> + tgs)) > > We have very too many whens in this function. It will be more succinct > to use a single (pcase org-refile-use-outline-path …) instead. Yes. But then I will be refactoring quite a lot of (working) code that I have not actually touching. I would prefer doing that in the separate patch. You’ve suggested some changes in my patches which could be applied to some other places in org-mode files I have seen. May be once we finish this discussion I would send a new series of patches with restyling? >> + ;; When `org-refile-use-outline-path’ is `title’, return extracted >> + ;; document title >> + (should >> + (equal ’(“T” “T/H1”) >> + (org-test-with-temp-text-in-file “#+title: T\n* H1” > > You may as well add a test when multiple #+title lines are present. Added. >> From 62684b478ae5ceb03f66967fbebcc4d6163c826c Mon Sep 17 00:00:00 2001 >> From: Mikhail Skorzhinskii >> Date: Sat, 12 Sep 2020 18:10:05 +0200 >> Subject: [PATCH 2/3] org-agenda.el: show document title in outline path > ^Show Fixed. >> * lisp/org.el (org-display-outline-path): Show a document title (#+TITLE >> value) and an outline path in an echo area if the customisation option >> is set to ’title. Fallback to a file or a buffer name if the document > ^ Fallback ;; (double space between sentences) Fixed. >> title is absent. > >> ** New options >> -*** New custom settings `org-icalendar-scheduled-summary-prefix' and `org-icalendar-deadline-summary-prefix' > > This is removing an existing NEWS entry. I guess it is not intentional. Yes. Sorry about that — fixed. >> +*** A new option for custom setting `org-agenda-show-outline-path' to show document title >> >> (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 outline path with prepended document >> +title. Fallback to file name is no title is present.” >> :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))) > > I think you can leave > (const :tag “Show outline path with prepended document title.” title) > > This text will be displayed in drop menu in cutomize interface alongside > with the full docstring. Mentioning the fallback in the docstring should > be good enough. Agreed. Fixed. >> From 5b15f886b22dc542220b48ae9659c4c2d56dea78 Mon Sep 17 00:00:00 2001 >> From: Mikhail Skorzhinskii >> Date: Thu, 8 Sep 2022 21:29:23 +0200 >> Subject: [PATCH 1/3] org-clock.el: rename org-clock-get-file-title > ^Rename Fixed. >> * lisp/org.el (org-get-title): A new function to collect a document >> title from an org-mode buffer, based on a org-clock-get-file-title >> implementation. > > `org-clock-get-file-title’. Elisp symbols should be quoted. Done. (on a side note, all my email clients show ` and ’ quite differently. I wonder why.) >> ** New functions and changes in function arguments >> +*** New function `org-get-title' to get `#+TITLE:' property from buffers > > We generally use `code' for Elisp symbols and `#+TITLE:' for verbatim > non-code text. (This has not been consistently followed in etc/NEWS, but > at least please change `#+TITLE' to `#+TITLE'). See > doc/Documentation_Standards.org Ah, yes. There are many occasions in the ORG-NEWS where this is not followed. Would you be interested in the patch fixing these irregularities? And if you do, would you prefer to have a fixed-up commits for these ones or just one big commit? I recently learned about existence of git absorb and couldn’t recommend it enough. >> ** Removed or renamed functions and variables >> +*** Rename `org-clock-get-file-title' to `org-get-file-title' >> + >> +This function is now part of the `org.el' file. > > You do not need to mention this. org-clock-get-file-title was > introduced in recent commits on main. Main is development branch, and we > do not need to document changes on the changes made after the last > release. OK. Fixed, left only a note about new function. I would say some users may find it interesting. I had a function that extracts title for many-many years. Used it for displaying document title in the frame title. >> ;;;###autoload >> (defun org-dblock-write:clocktable (params) >> “Write the standard clocktable.” >> @@ -2739,7 +2729,7 @@ from the dynamic block definition.“ >> ”\n“) >> >> (if filetitle >> - (org-clock-get-file-title file-name) >> + (org-get-file-title file-name) >> (file-name-nondirectory file-name)) >> (if level? ”| “ ”“) ;level column, maybe >> (if timestamp ”| “ ”“) ;timestamp column, maybe > > This may introduce a compiler warning. I suggest running make after > applying your patch and fix possible compiler warnings. (I suspect that > you may need to add declare-function on top of org-clock.el) Hm, I have tried it on the latest stable emacs (28.2) and it does not produce me a warning. `make compile' was just clean. Could you please refer me to the library/documentation why would I need to call `declare-function'? This is something from cl library? >> +(defun org-get-file-title (file-name) >> + “Collect title from `org-mode’ FILE-NAME. >> + >> +Return file name if title does not exist.” >> + (or (org-get-title (find-file-noselect file-name)) >> + (file-name-nondirectory file-name))) >> + >> +(defun org-get-title (&optional buffer) >> + “Collect title from the provided `org-mode’ BUFFER. >> + >> +Returns nil if there are no #+TITLE property.” >> + (let ((buffer (or (buffer-base-buffer) >> + buffer >> + (current-buffer)))) >> + (with-current-buffer buffer >> + (org-macro-initialize-templates) >> + (let ((title (assoc-default “title” org-macro-templates))) >> + (unless (string= “” title) >> + title))))) > > These two functions can be merged into a single `org-get-title’ that > accepts buffer or file-name as an optional argument. Merged, but a little bit differently. Now the function accepts buffer or file. Also, I removed the basebuffer logic, because this logic is already included in the `org-macro-initialize-templates'. Thanks, Mikhail Skorzhinskii