Hello, Rasmus <ras...@gmx.us> writes:
> These patches improve the block insertion mechanisms using both the > keyboard binding and org-tempo. Thank you. Some quick comments follow. > Patch 6 changes the key binding of blocks to "C-c C-," as discussed in > December 2017. Let me know if this key is OK and if the old key should > still be kept. I don't think the old key-binding should be kept. > +(defun org--mks-read-key (allowed-keys prompt) > + "Read a key and ensure it is a member of ALLOWED-KEYS. > + > +Tab, space and RET are treated equivalently." Nitpick: No need for a blank line in the docstring. Also: TAB, SPC and RET are ... > + (let* ((char (read-char-exclusive prompt)) > + (key (char-to-string > + (cond ((memq char '(?\s ?\t ?\r)) ?\t) > + (t char))))) Suggestion: (key (pcase (read-char-exclusive prompt) ((or ?\s ?\t ?\r) ?\t) (char char))) > + (let ((menu-choice (org--insert-structure-template-mks))) > + (if (equal (nth 0 menu-choice) "\t") > + (read-string "Structure type: ") > + (nth 1 menu-choice))))) (pcase (org--insert-structure-template-mks) (`("\t" . ,_) (read-string "Structure type: ")) (`(,_ ,choice . ,_) choice)) > (let* ((region? (use-region-p)) > (s (if region? (region-beginning) (point))) > (e (copy-marker (if region? (region-end) (point)) t)) > @@ -13568,7 +13652,7 @@ headlines matching this string." > ;; compile tags for current headline > (setq tags-list > (if org-use-tag-inheritance > - (apply 'append (mapcar 'cdr (reverse tags-alist))) > + (apply #'append (mapcar 'cdr (reverse tags-alist))) Nitpick: (mapcar #'cdr ...) > +(defun org-tempo--keys () > + (mapcar (lambda (pair) (format "<%s" (car pair))) > + (append org-structure-template-alist > + org-tempo-keywords-alist))) Missing docstring. > +(defun org-tempo--update-maybe () > + "Test if new tags have been added." I think you should clarify the docstring. It doesn't only test if new tags have been added, but it also add templates. Regards, -- Nicolas Goaziou