emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Ihor Radchenko <yantar92@gmail.com>
To: Arthur Miller <arthur.miller@live.com>
Cc: Max Nikulin <manikulin@gmail.com>,  emacs-orgmode@gnu.org
Subject: Re: Proposal: 'executable' org-capture-templaes
Date: Sat, 25 Jun 2022 15:32:20 +0800	[thread overview]
Message-ID: <87h749urjv.fsf@localhost> (raw)
In-Reply-To: <AM9PR09MB49771CF015DAECBF3E5F955E96B29@AM9PR09MB4977.eurprd09.prod.outlook.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


  parent reply	other threads:[~2022-06-25  7:34 UTC|newest]

Thread overview: 59+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-05-26 15:27 Proposal: 'executable' org-capture-templaes Arthur Miller
2022-05-27  5:27 ` Ihor Radchenko
2022-05-27 12:17   ` Arthur Miller
2022-05-27 14:35     ` Max Nikulin
2022-05-28  3:51     ` Ihor Radchenko
2022-05-30  2:04       ` Arthur Miller
2022-05-30  5:05         ` Ihor Radchenko
2022-05-30 12:40           ` Arthur Miller
2022-05-31  4:58             ` Ihor Radchenko
2022-05-31 14:46               ` Arthur Miller
2022-06-04 15:35               ` Arthur Miller
2022-06-05  0:04                 ` Ihor Radchenko
2022-06-05 15:16                   ` Arthur Miller
2022-06-05 23:05                     ` Tim Cross
2022-06-08 12:43                       ` Ihor Radchenko
2022-06-08 21:13                         ` Tim Cross
2022-06-09  4:00                           ` Ihor Radchenko
2022-06-17  4:40                         ` Arthur Miller
2022-06-18  4:03                           ` Ihor Radchenko
2022-06-18  4:26                             ` Tim Cross
2022-06-18 12:25                       ` Max Nikulin
2022-06-08 12:24                     ` Ihor Radchenko
2022-06-05  7:36                 ` Max Nikulin
2022-06-05 15:07                   ` Arthur Miller
2022-06-06 17:06                     ` Max Nikulin
2022-06-07  3:09                       ` Samuel Wales
2022-06-07  3:16                         ` Samuel Wales
2022-06-08 12:48                           ` Ihor Radchenko
2022-06-10 16:53                         ` Max Nikulin
2022-06-11  5:26                           ` Ihor Radchenko
2022-06-18  8:18                             ` Max Nikulin
2022-06-18  8:25                               ` Ihor Radchenko
2022-06-19 11:20                                 ` Max Nikulin
2022-06-20 12:10                                   ` Ihor Radchenko
2022-06-20 17:24                                     ` Max Nikulin
2022-06-21  4:07                                       ` Ihor Radchenko
2022-06-21  7:38                                         ` Arthur Miller
2022-06-21 15:48                                         ` Max Nikulin
2022-06-22 12:13                                           ` Arthur Miller
2022-06-22 16:29                                             ` Max Nikulin
2022-06-26  4:50                                               ` Arthur Miller
2022-06-29 17:02                                                 ` Max Nikulin
2022-06-30 23:30                                                   ` Arthur Miller
2022-07-01 15:53                                                     ` Proposal: 'executable' org-capture-templates Max Nikulin
2022-06-25  7:32                                             ` Ihor Radchenko [this message]
2022-06-26  4:25                                               ` Proposal: 'executable' org-capture-templaes Arthur Miller
2022-06-26  4:37                                                 ` Ihor Radchenko
2022-06-26  4:52                                                   ` Arthur Miller
2022-06-21  7:37                                       ` Arthur Miller
2022-07-02 11:31                                         ` Max Nikulin
2022-07-03 15:12                                           ` Arthur Miller
2022-07-07 16:14                                             ` Proposal: 'executable' org-capture-templates Max Nikulin
2022-06-18 15:05                               ` Proposal: 'executable' org-capture-templaes Arthur Miller
2022-06-19 10:53                                 ` Max Nikulin
2022-06-19 15:34                                   ` Arthur Miller
2022-07-03  3:32                                     ` Max Nikulin
2022-06-08 12:35                     ` Ihor Radchenko
2022-05-31 16:37         ` Max Nikulin
2022-06-01  1:45           ` arthur miller

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h749urjv.fsf@localhost \
    --to=yantar92@gmail.com \
    --cc=arthur.miller@live.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=manikulin@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).