First of all, thanks for providing a patch! However, please note that I cannot accept patches exceeding 15-20LOC without copyright assignment. Would you consider it? See https://orgmode.org/worg/org-contribute.html#copyright
Some comments on the patch below. > Subject: [PATCH] org-cite-basic-complete-key-crm-separator Please add a proper commit message, if you can. See https://orgmode.org/worg/org-contribute.html#commit-messages We also need an ORG-NEWS entry announcing the new customization. > +(defcustom org-cite-basic-complete-key-crm-separator nil > + "When non-nil, use `completing-read-multiple' with this as the separator. > +When nil: use traditional consecutive prompts. When set to a > +string (regexp), carefully set craft a regexp that doesn't appear in the "carefully set craft regexp..." reads as if Org will arrange this, not user. I think a simpler wording would be When nil, use multiple `completing-read' prompts. When set to a string, it should be a regexp to be used as `crm-separator' (which see). When set to symbol `dynamic', use ";;..." as a separator with the number of ";" sufficient so that none of the completion candidates contain the separator. > +(defun org-cite-basic--complete-key-dynamic-crm-separator > + (completion-hash-table separator) > + "Return a repeated version of SEPARATOR as needed that doesn't appear in > COMPLETION-HASH-TABLE." This docstring is too long. Please make sure that the first line of the docstring does not exceed 72 symbols. > + (let* ((cands (hash-table-keys completion-hash-table)) > + ;; Handle regexp separator? Is it a TODO? > + (dyn-sep separator) > + (consec-sep-regexp (format "\\(%s\\)+" separator))) regexp-opt will be safer if separator happens to be something like ".". > + (dlet ((crm-separator dlet is unnecessary here. A normal let will do because `crm-separator' is already a dynamically scoped variable. > + (cond > + ((stringp org-cite-basic-complete-key-crm-separator) > + org-cite-basic-complete-key-crm-separator) > + ((equal > + 'dynamic org-cite-basic-complete-key-crm-separator) > + (org-cite-basic--complete-key-dynamic-crm-separator > + table ";"))))) Nickpick: pcase of cl-case will be shorter >> We can use something like ;; as a separator or even compute it >> dynamically depending on the actual completion candidates (; if no >> candidates contain ;, ;;, ;;;, etc) > > How does this look? I haven't known a function to reliably query all > completion candidates from a completion table of regardless of its type. > > The ability to dynamically compute the crm-separator based on the > candidates sounds like a useful feature. I think it's best for Emacs > core to have that (optional) functionality built-in. We can discuss such addition on emacs-devel, if you are willing to. It will likely require more work though to make things more general. -- Ihor Radchenko // yantar92, Org mode maintainer, 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>
