RE: cal-tex.el landscape patch

["Vincent Belaïche" <vincent.b.1@hotmail.fr>]

Answers below.


De : Glenn Morris <rgm@gnu.org>
Envoyé : vendredi 25 août 2017 03:10
À : Vincent Belaïche
Cc : emacs-devel
Objet : Re: cal-tex.el landscape patch
> 
>
>Fine by me, thanks. Trivial comments below.
>
>> -(defun cal-tex-preamble (&optional args)
>> +(defun cal-tex-preamble (&optional class-options)
>>    "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 CLASS-OPTIONS are included as options for the article document class."
>>    (set-buffer (generate-new-buffer cal-tex-buffer))
>>    (insert (format "\\documentclass%s{article}\n"
>> -                  (if (stringp args)
>> -                      (format "[%s]" args)
>> +                  (if (stringp class-options)
>> +                      (format "[%s]" class-options)
>>                      "")))
>> +  (if (and (stringp class-options) (string-match "\\<landscape\\>" class-options))
>> +      (insert "\\usepackage{geometry}\n"))
>
>
>I'd prefer if the argument did not get renamed.

No problem, I will revert the prior name.
>
>
>> -(defun cal-tex-insert-preamble (weeks landscape size &optional append)
>> +(defun cal-tex-insert-preamble (weeks class-options &optional append)
>>    "Initialize the output LaTeX calendar buffer, `cal-tex-buffer'.
>>  Select the output buffer, and insert the preamble for a calendar
>> -of WEEKS weeks.  Insert code for landscape mode if LANDSCAPE is
>> -non-nil.  Use point-size SIZE.  Optional argument APPEND, if
>> -non-nil, means add to end of buffer without erasing current contents."
>> -  (let ((width "18cm")
>> +of WEEKS weeks.  Insert code for landscape mode if CLASS-OPTIONS
>> +contains landscape option. 
>> +Optional argument APPEND, if non-nil, means add to end of buffer
>> +without erasing current contents."
>
>"Insert code for landscape mode if CLASS-OPTIONS contains landscape
>option." seems a bit vague to me. Maybe something like
>
>"Pass string CLASS-OPTIONS as options for the article document class.
>If it contains \"landscape\", use the geometry package to produce
>landscape format."

You are fully right, your proposed docstring is quite more clear. I will
take it.
>
>Maybe class-options could be optional, and default to "12pt"?
>
It is already optional, I will make it default to "12pt", that is a good
idea.

  Vincent