Re: Proposal: 'executable' org-capture-templaes

[Ihor Radchenko <yantar92@gmail.com>]

Arthur Miller <arthur.miller@live.com> writes:

> I have reworked a bit org-capture, but I haven't yet worked on org-agenda
> restrictions and other details.

I do not think that you need to prioritize re-creating
org-capture/org-agenda/etc The main point is making sure that org-select
provides the required functionality.
I'd prefer to first finalize the API and get a cleaner code of
org-select itself.

> (define-minor-mode org-select-mode ""
>   :interactive nil :global nil)

Why don't you just make it a major mode derived from special-mode? It
will make more sense considering that you are setting special-mode,
keymap, etc.

> (defun osl--prop (property list)
>   "Return value of PROPERTY from irregular plist LIST."
>   (cadr (member property list)))

FYI, there is plist-get

> (defun osl--init ()
>   (buffer-local-value 'osl--init (current-buffer)))

This is no different than just saying osl--init.

> (defun org-select-abort ()
>   (interactive)
>   (org-select-quit "Aborted"))

Please make sure that all the functions and variables have docstrings.
This is not just a boring convention, it really helps when you come back
to the code in future and when other people are reading your code.

> (defun osl--longest-line ()
>   "Return the length of the longest line in current buffer."
>   (let ((n 1) (L 0) (e 0) (E (point-max)) l)
>     (while (< e E)
>       (setq e (line-end-position n)
>             l (- e (line-beginning-position n))
>             n (1+ n))
>       (if (> l L) (setq L l)))
>     L))

Please do not use single-char variable names for non-trivial variables.
It is always better to provide self-explanatory names. It is not a
programming context. We are targeting better readability, not fast
typing.

>       (dolist (menu menus)
>         (cond ((symbolp menu) (setq menu (eval menu)))
>               ((symbolp (car menu)) (setq menu (eval (car menu)))))
>         (let ((handler (osl--prop :org-select-handler menu)))
>           (when handler
>             (setq menu (delete :org-select-handler (delete handler menu))))

Destructive modifications of arguments is a bad idea. I expect future
bugs in such code. Please avoid this approach.

> ;; we send in the entire menu so we can return next piece in chain,
> ;; but *the* entry we work with is just the very first one (car menu)
> (defun osl--do-entry (menu handler)
>   "Display a single entry in the buffer."

AFAIU, the comment on top belongs to the docstring. At least the part
talking about the return value. If the function is expected to return
something, it should be documented. Otherwise, I expect bugs in future.

> (defun org-select-run (entry &optional _org-select-buffer)
>   "Try to execute form found in ENTRY if any leave ORG-SELECT-BUFFER live.
>
> This handler provides an easy way to use the framework for the simple
> use-cases for multiple choices. It relies on the user to press built-in choice
> `q' or `C-g' to exit the menu."

Please, do not use key bindings verbatim in docstring. Prefer commands.
Docstrings do have special format for auto-detecting command bindings.
See D.6 Tips for Documentation Strings section of Elisp manual.

>       ;; given a list of menus, display one menu at a time
>       (dolist (menu menus)
> 	(cond ((symbolp menu) (setq menu (eval menu)))
> 	      ((symbolp (car menu)) (setq menu (eval (car menu)))))

Please avoid eval when possible. It can behave not nicely in
lexical/dynamic scope.

> Each menu can be followed by some properties in form of a keu-value
> pair. The
                                                            ^key
> entire menu or entry does not need to be a regular plist. Following keys are
> recognized:
>
> :org-select-pin     Pin this menu in org-select buffer. If group nodes are used,
>                     when this option is `t', keep this menu visible even when
>                     descending into a submenu. ;; FIXME Not implemented yet.
> :org-select-handler Use this function to handle this particular menu or
>                     entry. When none is specified, org-select uses
>                     `org-select-run-once' to hande the menu. Entry handler
>                     takes precedence over menu handler.

This is confusing. What do you mean by "does not need to be a regular
plist"? What do you mean by "menu can be followed"? Do you imply that
MENUS may not be a list of MENU lists, but can also contain :key value
pairs?

In general, I'd prefer format similar to
https://github.com/progfolio/doct/
or at least similar to org-capture-templates
The global ARGS in org-select could be defined using cl-defun style. See
2.1 Argument Lists section of CL manual.

Best,
Ihor