Re: [AUCTeX-devel] Management of package options [Was: Re: Add `unit' type for the parser in siunitx.el]

[Tassilo Horn]

Mosè Giordano <address@hidden> writes:

Hi Mosè,

> I'm sending the whole patch set.

Thanks.  I have some questions and comments that we should address
before committing.

> It's a bit different from my previous patches, because after some
> testing I've noticed they didn't work well if one use
> `TeX-auto-generate-global' (many major GNU/Linux distros run it after
> installing AUCTeX package).  Now in `TeX-auto-store' I add elements to
> `LaTeX-provided-{class,package}-options' instead of setq'ing these
> variables, because in that case loading styles of included files would
> make a mess with the value of those variables.  I've defined a new
> function to add an alist to another.

That's fine with me.

> I've used `TeX-match-buffer' to remove text properties.  At the end of
> `LaTeX-arg-usepackage' I run style hooks for every active babel
> language.  A more general solution, applicable to packages other than
> babel, could be to reload inserted package if there are options.  This
> can be done using `TeX-unload-style', removing style from
> `TeX-active-styles' and running `TeX-run-style-hooks'.  But I believe
> that removing style from `TeX-active-styles' should be moved inside
> `TeX-unload-style', is it correct?  Is this solution acceptable?

Uh, sorry, I don't get you completely.  What I understand is that when
inserting

  \usepackage[german]{babel}

the change to `LaTeX-arg-usepackage' will also load german.  Why is that
needed?  AFAICS, loading babel will trigger loading of its active
languages, too.  See the hunk below:

> @@ -70,6 +79,9 @@
>  (TeX-add-style-hook
>   "babel"
>   (lambda ()
> +   ;; Run style hooks for every active language in loading order, so
> +   ;; `TeX-quote-language' will be correctly set.
> +   (mapc 'TeX-run-style-hooks (LaTeX-babel-active-languages))
>     ;; New symbols
>     (TeX-add-symbols
>      '("selectlanguage" TeX-arg-babel-lang)

Ok, some more nitpicking below:

> --- latex.el.old      2013-04-02 16:00:28.000000000 +0200
> +++ latex.el  2013-04-03 22:39:07.000000000 +0200
> @@ -1322,6 +1322,28 @@
>         (delete-backward-char 1)))))
>      opts))
>  
> +(defvar LaTeX-provided-class-options nil
> +  "Alist of options provided to LaTeX class.
> +For each element, the CAR is the name of the class, the CDR is
> +the list of options provided to it.")
> +(make-variable-buffer-local 'LaTeX-provided-class-options)

Please use the plural form "classes" in the docstring like you did with
"packages" below.

> +(defun LaTeX-provided-class-options-member (class option)
> +  "Return non-nil if OPTION has been given to CLASS at load time.
> +The value is actually the tail of the list of options given to CLASS."
> +  (member option (cdr (assoc package LaTeX-provided-class-options))))
> +
> +(defvar LaTeX-provided-package-options nil
> +  "Alist of options provided to LaTeX packages.
> +For each element, the CAR is the name of the package, the CDR is
> +the list of options provided to it.")
> +(make-variable-buffer-local 'LaTeX-provided-package-options)

And please add some example like

  (("babel" "german")
   ("geometry" "a4paper" "top=2cm" "bottom=2cm" "left=2.5cm" "right=2.5cm")
   ...)
     
to the docstring.  Ditto for the class options variable.  At least for
me, that's easier to grasp than a plain textual description.

> +        ;; Treat documentclass/documentstyle specially.
> +        (if (or (string-equal "package" class)
> +                (string-equal "Package" class))
> +            (dolist (elt (TeX-split-string
> +                          "\\([ \t\r\n]\\|%[^\n\r]*[\n\r]\\|,\\)+" style))
> +           ;; Append style to the style list.
> +           (add-to-list 'TeX-auto-file elt t)
> +              ;; Append to `LaTeX-provided-package-options' the name of the
> +              ;; package and the options provided to it at load time.
> +           (unless (equal options '(""))
> +             (TeX-add-to-alist 'LaTeX-provided-package-options
> +                               (list (add-to-list 'options elt)))))

Isn't (list (cons elt options)) better?  The `add-to-list' might be bad
because it changes the value of `options', and that is used afterwards
again.  Also, it won't do what you expect in case some package has an
option named as the package itself.

>         ;; And a special "art10" style file combining style and size.
>         (add-to-list 'TeX-auto-file style t)
>         (add-to-list 'TeX-auto-file
> @@ -1385,7 +1411,10 @@
>                             ((member "12pt" options)
>                              "12")
>                             (t
> -                            "10"))) t))
> +                            "10"))) t)
> +       (unless (equal options '(""))
> +         (TeX-add-to-alist 'LaTeX-provided-class-options
> +                           (list (add-to-list 'options style)))))

Same thing here.

Bye,
Tassilo