emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: cal-tex.el landscape patch


From: Glenn Morris
Subject: Re: cal-tex.el landscape patch
Date: Mon, 28 Aug 2017 13:55:59 -0400
User-agent: Gnus (www.gnus.org), GNU Emacs (www.gnu.org/software/emacs/)

Thanks, comments below.

> --- a/lisp/calendar/cal-tex.el
> +++ b/lisp/calendar/cal-tex.el
> @@ -259,12 +259,37 @@ cal-tex-list-diary-entries
>  (defun cal-tex-preamble (&optional args)
>    "Insert the LaTeX calendar preamble into `cal-tex-buffer'.
>  Preamble includes initial definitions for various LaTeX commands.
> -Optional string ARGS are included as options for the article document class."
> +Optional string ARGS are included as options for the article
> +document class with inclusion of default values \"12pt\" for
> +size, and \"a4paper\" for paper unless size or paper are already
> +specified in ARGS.  When ARGS is omitted, by default the option
> +\"12pt,a4paper\" is passed.

I think in hindsight my suggestion to default to 12pt was not a good
one, because it makes the argument parsing ugly. Sorry.
And please don't default to A4 paper, since the default locale for Emacs is
the US one (spelling etc).

> +Please note that if ARGS is \"\" then
> +\"\\documentclass[]{article}\" is inserted, while if ARGS it `t'
> +then \"\\documentclass{article}\" is inserted."

This doesn't seem like a nice interface.
Why do you need the ARGS t case when it can be nil?

>    (set-buffer (generate-new-buffer cal-tex-buffer))
> -  (insert (format "\\documentclass%s{article}\n"
> -                  (if (stringp args)
> -                      (format "[%s]" args)
> -                    "")))
> +  (save-match-data
> +    (insert (format "\\documentclass%s{article}\n"
> +                    (cond
> +                     ((stringp args)
> +                      ;; set default size
> +                      (unless (string-match "\\(^\\|,\\) *[0-9]+pt 
> *\\(,\\|$\\)" args)
> +                        (setq args (concat args ",12pt")))
> +                      ;; set default paper
> +                      (unless (string-match "\\(^\\|,\\) 
> *\\([ab][4-5]\\|le\\(tter\\|gal\\)\\|executive\\)paper *\\(,\\|$\\)" args)
> +                        (setq args (concat args ",a4paper")))
> +                      (when (string= (substring args 0 1) ",")
> +                        (setq args (substring args 1)))
> +                      (format "[%s]" args))
> +                     ((null args) "[12pt]")
> +                     (t ""))))

This seems overly complicated to me.
Again, it wasa bad suggestion of mine to have default values.
string-match-p would avoid the need to save-match-data.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]