Re: [AUCTeX-devel] Adding key/vals to predefined ones

[Arash Esbati]

Tassilo Horn <tsdh <at> gnu.org> writes:

Hi Tassilo,

> Ah, yes.  But I still have some question about that function.
> 
> --8<---------------cut here---------------start------------->8---
> (defun LaTeX-enumitem-update-key-val-options ()
>   "Update the buffer-local key-val options before offering them
> in `enumitem'-completions."
>   (unless (null LaTeX-enumitem-SetEnumitemKey-list)
>     (dolist (key (apply 'append LaTeX-enumitem-SetEnumitemKey-list))
>       (add-to-list 'LaTeX-enumitem-key-val-options-local (list key))))
>   (unless (null LaTeX-enumitem-SetEnumitemValue-list)
>     (dolist (keyvals (apply 'append LaTeX-enumitem-SetEnumitemValue-list))
>       (let* ((key (car keyvals))
>            (val (cadr keyvals))
>            ;; (key-match (car (assoc key 
> LaTeX-enumitem-key-val-options-local)))
>            (val-match (cdr (assoc key LaTeX-enumitem-key-val-options-local)))
>            (temp  (copy-alist LaTeX-enumitem-key-val-options-local))
>            (opts (assq-delete-all (car (assoc key temp)) temp)))
>       (if (null val-match)
>           (add-to-list 'opts (list key (list val)))
>         (add-to-list 'opts
>                      (list key (delete-dups (apply 'append (list val) 
> val-match)))))
>       (setq LaTeX-enumitem-key-val-options-local (copy-alist opts))))))
> --8<---------------cut here---------------end--------------->8---
> 
> Why the (unless (null ...) ...) checks?  That's the same as (when ...)
> but doubly negated, no?  And why do you need that anyhow?  The (dolist
> ...) doesn't need it.

My idea was to gain some speed by checking if
`LaTeX-enumitem-SetEnumitem(Key|Value)-list' are changed and then go through
the (dolist ...) action.  I'm not sure how popular these features of
`enumitem' are.

> Oh, and I guess the (apply 'append ...) is for flattening the *-list
> vars.  Instead, please use the functions of the same name which return
> the keys/key-vals in a uniform format so you don't need those hacks.

I had that but it turned out that the function swallows items, example:

--8<---------------cut here---------------start------------->8---
\documentclass{article}
\usepackage[inline]{enumitem}

\SetEnumitemValue{label}{numeric}{ignore}
\SetEnumitemValue{leftmargin}{standard}{ignore}
\SetEnumitemValue{align}{flushX}{ignore}
\SetEnumitemValue{align}{flushY}{ignore}
\SetEnumitemValue{align}{flushZ}{ignore}

(LaTeX-enumitem-SetEnumitemValue-list)

\begin{document} foo \end{document}
--8<---------------cut here---------------end--------------->8---

After `C-c C-n', `C-h v' on `LaTeX-enumitem-SetEnumitemValue-list' returns:

Its value is ((("label" "numeric")
  ("leftmargin" "standard")
  ("align" "flushX")
  ("align" "flushY")
  ("align" "flushZ"))
 nil)

Doing `C-x C-e' on `(LaTeX-enumitem-SetEnumitemValue-list)' returns:

(("align" "flushX") ("label" "numeric") ("leftmargin" "standard"))

and a new `C-h v' on `LaTeX-enumitem-SetEnumitemValue-list' returns:

Its value is (("align" "flushX")
 ("label" "numeric")
 ("leftmargin" "standard"))

Am I missing something?

> And please don't use `add-to-list' on local variables.  Instead, use
> `pushnew' (with :test #'string= in case its a list of strings), e.g.,
> 
>   (pushnew opts (list key (list val)) :test #'string=)
> 
> instead of
> 
>   (add-to-list 'opts (list key (list val)))

Thnx, I should have read the doc-string from `add-to-list' more carefully. 
I have now,

   (cl-pushnew (list key (list val)) opts :test #'equal)

> And lastly, in general I prefer the test expression of (if test ...) to
> be positively defined, at least when both then and else are just one
> single form.  So (if val-match ...) instead (if (null val-match) ...)
> above.

The whole function looks like this now:

--8<---------------cut here---------------start------------->8---
(defun LaTeX-enumitem-update-key-val-options ()
  "Update the buffer-local key-val options before offering them
in `enumitem'-completions."
  (when LaTeX-enumitem-SetEnumitemKey-list
    (dolist (key (LaTeX-enumitem-SetEnumitemKey-list))
      (add-to-list 'LaTeX-enumitem-key-val-options-local key)))
  (when LaTeX-enumitem-SetEnumitemValue-list
    (dolist (keyvals (apply 'append LaTeX-enumitem-SetEnumitemValue-list))
      (let* ((key (car keyvals))
             (val (cadr keyvals))
             ;; (key-match (car (assoc key 
LaTeX-enumitem-key-val-options-local)))
             (val-match (cdr (assoc key LaTeX-enumitem-key-val-options-local)))
             (temp  (copy-alist LaTeX-enumitem-key-val-options-local))
             (opts (assq-delete-all (car (assoc key temp)) temp)))
        (if val-match
            (cl-pushnew (list key (delete-dups (apply 'append (list val) 
val-match)))
                        opts :test #'equal)
          (cl-pushnew (list key (list val)) opts :test #'equal))
        (setq LaTeX-enumitem-key-val-options-local (copy-alist opts))))))
--8<---------------cut here---------------end--------------->8---

> Could you please adapt your function above and the rest of the code
> accordingly and then submit a complete patch?  I guess we're good to go
> then. 

I will adapt the rest this week, hopefully I can make a patch next weekend.

Thank you for your patience.
Best, Arash