[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [patch] Snippet expansion
From: |
Nicolas Goaziou |
Subject: |
Re: [O] [patch] Snippet expansion |
Date: |
Sun, 24 Dec 2017 16:32:10 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) |
Hello,
Rasmus <address@hidden> writes:
> The first patches adds string keys to snippet expansion. For tempo, this
> is straight-forward.
>
> For the interactive prompt there’s an org-mks interface. It limited to at
> most two keys (this shouldn’t be much of a limitation TBH). So for
> instance if the key is "prop" the interactive prompt will be "p", "pr",
> "po" or "pp".
>
> The second patch are various improvements for org-tempo, e.g. to put the
> cursor at a better place. org-tempo now also do some checks wrt. new
> structure templates.
>
> The third patch is more experimental and tries to be more "clever" when
> using the interactive interface, e.g. with newlines. The code is now a
> lot more complicated, and I’m not sure it’s any more pleasant or
> DWIMish...
Thank you. Some comments follow (but none about usability).
> (defcustom org-tempo-keywords-alist
> - '((?L . "latex")
> - (?H . "html")
> - (?A . "ascii")
> - (?i . "index"))
> + '(("L" . "latex")
> + ("H" . "html")
> + ("A" . "ascii")
> + ("i" . "index"))
You need to update :type keyword.
> (defcustom org-structure-template-alist
> - '((?a . "export ascii")
> - (?c . "center")
> - (?C . "comment")
> - (?e . "example")
> - (?E . "export")
> - (?h . "export html")
> - (?l . "export latex")
> - (?q . "quote")
> - (?s . "src")
> - (?v . "verse"))
> + '(("a" . "export ascii")
> + ("c" . "center")
> + ("C" . "comment")
> + ("e" . "example")
> + ("E" . "export")
> + ("h" . "export html")
> + ("l" . "export latex")
> + ("q" . "quote")
> + ("s" . "src")
> + ("v" . "verse"))
Ditto.
> +(autoload 'org-mks "org-capture" "Select a member of an alist with multiple
> keys." t)
If we're going to use "org-mks", I suggest to first move "org-mks" into
"org-macs.el" (which is the generic Org toolbox) first, then adapt
callers.
> +(defun org-insert-structure-template--mks ()
"--" is put after library prefix. Since that's org, it should be
"org--insert-structure-template-mks".
> + "Present `org-structure-template-alist' with `org-mks'.
> +
> +- Menus are added if keys require more than one stroke.
> +- Tabs are added to single key entires when needing more than one stroke.
> +- Keys longer than two characters are reduced to two characters."
> + (let* (case-fold-search
> + (keys (mapcar 'car org-structure-template-alist))
Nitpick: 'car -> #'car
> + (start-letters (delete-dups (mapcar (lambda (key) (substring key 0
> 1)) keys)))
> + (mks (mapcar (lambda (letter)
> + (list letter
> + (cl-remove-if-not
> + (apply-partially 'string-match-p (concat "^"
> letter))
'string-match-p -> #'string-match-p
> + org-structure-template-alist :key 'car)))
Ditto.
> + start-letters)))
> + (org-mks
> + (apply 'append
Ditto.
> + (mapcar (lambda (sublist)
> + (if (eq 1 (length (cadr sublist)))
> + (mapcar (lambda (elm)
> + (list (substring (car elm) 0 1)
> + (cdr elm) ""))
> + (cadr sublist))
> + (let* ((topkey (car sublist))
> + (elms (cadr sublist))
> + (keys (mapcar 'car elms))
> + (longp (> (length elms) 3)))
Nitpick: longp -> long?
`longp' is not a predicate (i.e., a function).
> + (append
> + (list (list topkey
> + (concat
> + (mapconcat 'cdr
Guess what.
> + (cl-subseq elms 0 (if longp
> 3 (length elms)))
> + ", ")
> + (when longp ", ..."))))
> + (cl-mapcar 'list
Ahem.
> +(defun org-insert-structure-template--unique-keys (keys)
> + "Make each key in KEYS unique and two characters long.
> +
> +For keys more than two characters, find the first unique
> +combination of the first letter and subsequent letters."
> + (cl-loop for key in keys
> + ;; There should at most be one key that is of length one.
> + if (eq 1 (length key))
> + collect (concat key "\t") into new-keys
> + ;; All keys of two characters should be unique.
> + else if (eq (length key) 2)
> + collect key into new-keys
> + else
> + collect
> + (cl-find-if-not (lambda (k) (member k new-keys))
> + (mapcar (apply-partially 'concat (substring key 0
> 1))
> + (split-string (substring key 1) "" t)))
> + into new-keys
> + finally return new-keys))
This looks overly complicated. Why not simply iterating over keys and
maintaining a list of keys stored so far, rejecting any duplicate along
the way?
> +(defun org-tempo-complete-tag ()
> + (org-tempo--update-maybe)
> + (call-interactively 'tempo-complete-tag))
*coughs*
> + (indent (make-string col ? ))
"? " -> "?\s"
> + (when region?
> + (while (looking-at-p "^[ \t]*$")
> + (delete-region (line-beginning-position)
> + (1+ (line-end-position)))))
(1+ (line-end-position)) -> (line-beginning-position 2)
The former can be greater than (point-max).
> + (save-excursion
> + (while (not (eobp))
> + (unless (looking-at-p indent)
> + (insert indent))
> + (forward-line)))
> + (insert
> + indent
> + (format "#+begin_%s%s\n" type (if specialp " " "")))
specialp -> special? (this is not a predicate).
Regards,
--
Nicolas Goaziou