cal-tex.el landscape patch

cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1: Type: text/plain, Size: 612 bytes --]

Hello,

Not being the maintainer of calendar, I would like to get your
approval/brickbats/comments before commiting this : the objective is to
make landscape by landscape class option + \usepackage{geometry}, rather
than \special{landscape} in the preamble.

The advantage of this way is that this it works directly if you compile
with pdflatex, rather than latex+dvips+ps2pdf.  To achieve this I had to
change some of the function prototypes, the landscape argument is
removed, and landscape option is passed just as another class option.

BR,
  Vincent.

PS : This would go in the master branch.

[-- Attachment #2: cal-tex.diff.txt --]
[-- Type: text/plain, Size: 4533 bytes --]

diff --git a/lisp/calendar/cal-tex.el b/lisp/calendar/cal-tex.el
index 1ea10bf..3abc53f 100644
--- a/lisp/calendar/cal-tex.el
+++ b/lisp/calendar/cal-tex.el
@@ -256,15 +256,17 @@ cal-tex-list-diary-entries
     (diary-list-entries (calendar-gregorian-from-absolute d1)
                         (1+ (- d2 d1)) t)))
 
-(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"))
   (if (stringp cal-tex-preamble-extra)
       (insert cal-tex-preamble-extra "\n"))
   ;; FIXME boxwidth and boxheight unused?
@@ -320,7 +322,7 @@ cal-tex-year
 There are four rows of three months each, unless optional
 LANDSCAPE is non-nil, in which case the calendar is printed in
 landscape mode with three rows of four months each."
-  (cal-tex-insert-preamble 1 landscape "12pt")
+  (cal-tex-insert-preamble 1 (if landscape "12pt,landscape" "12pt"))
   (if landscape
       (cal-tex-vspace "-.6cm")
     (cal-tex-vspace "-3.1cm"))
@@ -476,7 +478,7 @@ cal-tex-cursor-month-landscape
          (diary-list (if cal-tex-diary (cal-tex-list-diary-entries d1 d2)))
          (holidays (if cal-tex-holidays (holiday-in-range d1 d2)))
          other-month other-year small-months-at-start)
-    (cal-tex-insert-preamble (cal-tex-number-weeks month year 1) t "12pt")
+    (cal-tex-insert-preamble (cal-tex-number-weeks month year 1) "12pt,landscape")
     (cal-tex-cmd cal-tex-cal-one-month)
     (dotimes (i n)
       (setq other-month month
@@ -515,7 +517,7 @@ cal-tex-cursor-month-landscape
         (calendar-increment-month month year 1)
         (cal-tex-vspace "-2cm")
         (cal-tex-insert-preamble
-         (cal-tex-number-weeks month year 1) t "12pt" t))))
+         (cal-tex-number-weeks month year 1) "12pt,landscape" t))))
   (cal-tex-end-document)
   (run-hooks 'cal-tex-hook))
 
@@ -545,7 +547,7 @@ cal-tex-cursor-month
                       end-year))))
          (diary-list (if cal-tex-diary (cal-tex-list-diary-entries d1 d2)))
          (holidays (if cal-tex-holidays (holiday-in-range d1 d2))))
-    (cal-tex-insert-preamble (cal-tex-number-weeks month year n) nil "12pt")
+    (cal-tex-insert-preamble (cal-tex-number-weeks month year n) "12pt")
     (if (> n 1)
         (cal-tex-cmd cal-tex-cal-multi-month)
       (cal-tex-cmd cal-tex-cal-one-month))
@@ -1615,24 +1617,26 @@ cal-tex-end-document
 \t\tM-x tex-buffer RET
 \t\tM-x tex-print  RET")))
 
-(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."
+  (let ((landscape  (and class-options
+                         (string-match "\\<landscape\\>" class-options)))
+        (width "18cm")
         (height "24cm"))
     (when landscape
-      (setq width "24cm"
-            height "18cm"))
+      (let ((swap  width))
+       (setq width height height swap)))
     (unless append
-      (cal-tex-preamble size)
+      (cal-tex-preamble class-options)
       (if (not landscape)
           (progn
             (cal-tex-cmd "\\oddsidemargin -1.75cm")
             (cal-tex-cmd "\\def\\holidaymult" ".06"))
-        (cal-tex-cmd "\\special" "landscape")
         (cal-tex-cmd "\\textwidth 9.5in")
         (cal-tex-cmd "\\textheight 7in")
         (cal-tex-comment)

Re: cal-tex.el landscape patch

[Marcin Borkowski]

On 2017-08-21, at 16:19, Vincent Belaïche <vincent.b.1@hotmail.fr> wrote:

> Hello,
>
> Not being the maintainer of calendar, I would like to get your
> approval/brickbats/comments before commiting this : the objective is to
> make landscape by landscape class option + \usepackage{geometry}, rather
> than \special{landscape} in the preamble.
>
> The advantage of this way is that this it works directly if you compile
> with pdflatex, rather than latex+dvips+ps2pdf.  To achieve this I had to
> change some of the function prototypes, the landscape argument is
> removed, and landscape option is passed just as another class option.

+1

And my 2 cents:

Not being the maintainer either, but sometimes a user, I'm for it.
However, I feel that cal-tex requires a general overhaul.  The LaTeX
file it produces doesn't look really great (neither the source nor the
output).  And it doesn't fit on A4.

I could try to do that overhaul.  For starters, enable A4 calendars;
also, why not use TikZ to draw the boxes (at least as an option)?  WDYT?

Best,

--
Marcin Borkowski

RE: cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1: Type: text/plain, Size: 1829 bytes --]

Hello,

Yes I fully agree that there is room to improve it.

For fitting to the paper dimension, the simplest would be to have the LaTeX code derive box sizes (\cellwidth...) from \textheight & \textwidth, and derive in turn \textheight & \textwidth from \paperheight & \paperwidth and the wanted margin. All of this can be done in the LaTeX code.

Another area of improvement would be to replace \usepackage[latin1]{inputenc} by the adquate encoding when holidays are used, or week days name are not English.


  Vincent.

________________________________
De : Marcin Borkowski <mbork@mbork.pl>
Envoyé : mardi 22 août 2017 09:04
À : Vincent Belaïche
Cc : emacs-devel
Objet : Re: cal-tex.el landscape patch


On 2017-08-21, at 16:19, Vincent Belaïche <vincent.b.1@hotmail.fr> wrote:

> Hello,
>
> Not being the maintainer of calendar, I would like to get your
> approval/brickbats/comments before commiting this : the objective is to
> make landscape by landscape class option + \usepackage{geometry}, rather
> than \special{landscape} in the preamble.
>
> The advantage of this way is that this it works directly if you compile
> with pdflatex, rather than latex+dvips+ps2pdf.  To achieve this I had to
> change some of the function prototypes, the landscape argument is
> removed, and landscape option is passed just as another class option.

+1

And my 2 cents:

Not being the maintainer either, but sometimes a user, I'm for it.
However, I feel that cal-tex requires a general overhaul.  The LaTeX
file it produces doesn't look really great (neither the source nor the
output).  And it doesn't fit on A4.

I could try to do that overhaul.  For starters, enable A4 calendars;
also, why not use TikZ to draw the boxes (at least as an option)?  WDYT?

Best,

--
Marcin Borkowski

[-- Attachment #2: Type: text/html, Size: 3039 bytes --]

Re: cal-tex.el landscape patch

[Glenn Morris]

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.


> -(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."

Maybe class-options could be optional, and default to "12pt"?

RE: cal-tex.el landscape patch

[Vincent Belaïche]

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

RE: cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1: Type: text/plain, Size: 2927 bytes --]

Oops... sorry for my mistake, class-options is not optional in cal-tex-insert-preamble. I will make it options as you suggest.


  Vincent.


________________________________
De : Vincent Belaïche <vincent.b.1@hotmail.fr>
Envoyé : vendredi 25 août 2017 11:07
À : Glenn Morris
Cc : emacs-devel
Objet : RE: cal-tex.el landscape patch


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<file://\\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<file://\\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

[-- Attachment #2: Type: text/html, Size: 5235 bytes --]

Re: cal-tex.el landscape patch

[Edward Reingold]

[-- Attachment #1: Type: text/plain, Size: 3208 bytes --]

I would add easy optional use of colors for, say, holidays (red) and/or
diary entries (blue).

 Fri, Aug 25, 2017 at 12:59 PM Vincent Belaïche <vincent.b.1@hotmail.fr>
wrote:

> Oops... sorry for my mistake, class-options is not optional in
> cal-tex-insert-preamble. I will make it options as you suggest.
>
>
>   Vincent.
> ------------------------------
> *De :* Vincent Belaïche <vincent.b.1@hotmail.fr>
> *Envoyé :* vendredi 25 août 2017 11:07
> *À :* Glenn Morris
> *Cc :* emacs-devel
> *Objet :* RE: cal-tex.el landscape patch
>
>
> 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
>

[-- Attachment #2: Type: text/html, Size: 5934 bytes --]

RE: cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1: Type: text/plain, Size: 4000 bytes --]

That is a great idea. The LaTeX code generated by cal-tex.el could have some formatter macro defined by \providecommand as identity by default. Then the user could change the definition by inserting some custom LaTeX code that would for instance define the formatter as #1->\textcolor{red}{#1}.


Another idea is that the code generator for the diary could cusotomized to use OrgMode translation instead of cal-tex-LaTeXify-string, so you would write in the diary an entry as :


@@latex:\textcolor{red}{@@My important appointment@@latex:}@@


This second idea would make it easier to have different colors or formatting for different items.


   Vincent.

________________________________
De : Edward Reingold <reingold@iit.edu>
Envoyé : vendredi 25 août 2017 20:25:51
À : Glenn Morris; Vincent Belaïche
Cc : emacs-devel
Objet : Re: cal-tex.el landscape patch

I would add easy optional use of colors for, say, holidays (red) and/or diary entries (blue).

 Fri, Aug 25, 2017 at 12:59 PM Vincent Belaïche <vincent.b.1@hotmail.fr<mailto:vincent.b.1@hotmail.fr>> wrote:

Oops... sorry for my mistake, class-options is not optional in cal-tex-insert-preamble. I will make it options as you suggest.


  Vincent.

________________________________
De : Vincent Belaïche <vincent.b.1@hotmail.fr<mailto:vincent.b.1@hotmail.fr>>
Envoyé : vendredi 25 août 2017 11:07
À : Glenn Morris
Cc : emacs-devel
Objet : RE: cal-tex.el landscape patch


Answers below.


De : Glenn Morris <rgm@gnu.org<mailto: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

[-- Attachment #2: Type: text/html, Size: 8103 bytes --]

RE: cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1.1: Type: text/plain, Size: 5457 bytes --]

Hello,

Here is the my updated patch, and below the change log. If every body agrees I can commit/push the change.


I have also made a4paper a default option, because if in the future we want to support other paper sizes, and the cellwidth and so on computed by LaTeX code based on \papwerwidth, then the paper size has to be specified always.


ChangeLog :

    * lisp/calendar/cal-tex.el (cal-tex-preamble): Make 12pt the
    default class option. Passing no option, not even an empty
    option list, can still be done by passing `t' for args.
    (cal-tex-year, cal-tex-cursor-month-landscape): Pass landscape
    request to `cal-tex-insert-preamble' function call within the
    class option string.
    (cal-tex-cursor-month): Don't pass any longer "12pt" argument
    to `cal-tex-insert-preamble' function, as it is default.
    (cal-tex-insert-preamble): Suppress landscape and size
    argument, and replace them by a class-options string
    argument. Do not insert any longer "\special{landscape}" in
    case of landscape layout, as the job is made by the geometry
    package.
    (cal-tex-cursor-week): Don't pass any longer "12pt" argument
    to `cal-tex-preamble' function, as it is default.


________________________________
De : Vincent Belaïche <vincent.b.1@hotmail.fr>
Envoyé : samedi 26 août 2017 17:06:46
À : Edward Reingold; Glenn Morris
Cc : emacs-devel
Objet : RE: cal-tex.el landscape patch


That is a great idea. The LaTeX code generated by cal-tex.el could have some formatter macro defined by \providecommand as identity by default. Then the user could change the definition by inserting some custom LaTeX code that would for instance define the formatter as #1->\textcolor{red}{#1}.


Another idea is that the code generator for the diary could cusotomized to use OrgMode translation instead of cal-tex-LaTeXify-string, so you would write in the diary an entry as :


@@latex:\textcolor{red}{@@My important appointment@@latex:}@@


This second idea would make it easier to have different colors or formatting for different items.


   Vincent.

________________________________
De : Edward Reingold <reingold@iit.edu>
Envoyé : vendredi 25 août 2017 20:25:51
À : Glenn Morris; Vincent Belaïche
Cc : emacs-devel
Objet : Re: cal-tex.el landscape patch

I would add easy optional use of colors for, say, holidays (red) and/or diary entries (blue).

 Fri, Aug 25, 2017 at 12:59 PM Vincent Belaïche <vincent.b.1@hotmail.fr<mailto:vincent.b.1@hotmail.fr>> wrote:

Oops... sorry for my mistake, class-options is not optional in cal-tex-insert-preamble. I will make it options as you suggest.


  Vincent.

________________________________
De : Vincent Belaïche <vincent.b.1@hotmail.fr<mailto:vincent.b.1@hotmail.fr>>
Envoyé : vendredi 25 août 2017 11:07
À : Glenn Morris
Cc : emacs-devel
Objet : RE: cal-tex.el landscape patch


Answers below.


De : Glenn Morris <rgm@gnu.org<mailto: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

[-- Attachment #1.2: Type: text/html, Size: 10412 bytes --]

[-- Attachment #2: cal-tex-1.diff.txt --]
[-- Type: text/plain, Size: 5948 bytes --]

diff --git a/lisp/calendar/cal-tex.el b/lisp/calendar/cal-tex.el
index 1ea10bf..09cdb3b 100644
--- 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. When ARGS has any other value, then
+no option is passed to the class.
+
+Insert the \"\\usepacakge{geometry}\" directive when ARGS
+contains the \"landscape\" string.
+
+Please note that if ARGS is \"\" then
+\"\\documentclass[]{article}\" is inserted, while if ARGS it `t'
+then \"\\documentclass{article}\" is inserted."
   (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 ""))))
+    (if (and (stringp args) (string-match "\\<landscape\\>" args))
+      (insert "\\usepackage{geometry}\n")))
   (if (stringp cal-tex-preamble-extra)
       (insert cal-tex-preamble-extra "\n"))
   ;; FIXME boxwidth and boxheight unused?
@@ -320,7 +345,7 @@ cal-tex-year
 There are four rows of three months each, unless optional
 LANDSCAPE is non-nil, in which case the calendar is printed in
 landscape mode with three rows of four months each."
-  (cal-tex-insert-preamble 1 landscape "12pt")
+  (cal-tex-insert-preamble 1 (and landscape "landscape"))
   (if landscape
       (cal-tex-vspace "-.6cm")
     (cal-tex-vspace "-3.1cm"))
@@ -476,7 +501,7 @@ cal-tex-cursor-month-landscape
          (diary-list (if cal-tex-diary (cal-tex-list-diary-entries d1 d2)))
          (holidays (if cal-tex-holidays (holiday-in-range d1 d2)))
          other-month other-year small-months-at-start)
-    (cal-tex-insert-preamble (cal-tex-number-weeks month year 1) t "12pt")
+    (cal-tex-insert-preamble (cal-tex-number-weeks month year 1) "landscape")
     (cal-tex-cmd cal-tex-cal-one-month)
     (dotimes (i n)
       (setq other-month month
@@ -515,7 +540,7 @@ cal-tex-cursor-month-landscape
         (calendar-increment-month month year 1)
         (cal-tex-vspace "-2cm")
         (cal-tex-insert-preamble
-         (cal-tex-number-weeks month year 1) t "12pt" t))))
+         (cal-tex-number-weeks month year 1) "landscape" t))))
   (cal-tex-end-document)
   (run-hooks 'cal-tex-hook))
 
@@ -545,7 +570,7 @@ cal-tex-cursor-month
                       end-year))))
          (diary-list (if cal-tex-diary (cal-tex-list-diary-entries d1 d2)))
          (holidays (if cal-tex-holidays (holiday-in-range d1 d2))))
-    (cal-tex-insert-preamble (cal-tex-number-weeks month year n) nil "12pt")
+    (cal-tex-insert-preamble (cal-tex-number-weeks month year n))
     (if (> n 1)
         (cal-tex-cmd cal-tex-cal-multi-month)
       (cal-tex-cmd cal-tex-cal-one-month))
@@ -739,7 +764,7 @@ cal-tex-cursor-week
          (d2 (+ (* 7 n) d1))
          (holidays (if cal-tex-holidays
                        (holiday-in-range d1 d2))))
-    (cal-tex-preamble "11pt")
+    (cal-tex-preamble)
     (cal-tex-weekly-paper)
     (insert cal-tex-LaTeX-hourbox)
     (cal-tex-b-document)
@@ -1615,24 +1640,27 @@ cal-tex-end-document
 \t\tM-x tex-buffer RET
 \t\tM-x tex-print  RET")))
 
-(defun cal-tex-insert-preamble (weeks landscape size &optional append)
+(defun cal-tex-insert-preamble (weeks &optional class-options 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.  Pass string CLASS-OPTIONS as options for the
+article document class.  If it contains \"landscape\", use the
+geometry package to produce landscape format.  Optional argument
+APPEND, if non-nil, means add to end of buffer without erasing
+current contents."
+  (let ((landscape  (and class-options
+                         (string-match "\\<landscape\\>" class-options)))
+        (width "18cm")
         (height "24cm"))
     (when landscape
-      (setq width "24cm"
-            height "18cm"))
+      (let ((swap  width))
+       (setq width height height swap)))
     (unless append
-      (cal-tex-preamble size)
+      (cal-tex-preamble class-options)
       (if (not landscape)
           (progn
             (cal-tex-cmd "\\oddsidemargin -1.75cm")
             (cal-tex-cmd "\\def\\holidaymult" ".06"))
-        (cal-tex-cmd "\\special" "landscape")
         (cal-tex-cmd "\\textwidth 9.5in")
         (cal-tex-cmd "\\textheight 7in")
         (cal-tex-comment)

Re: cal-tex.el landscape patch

[Paul Eggert]

On 08/26/2017 08:29 AM, Vincent Belaïche wrote:
> I have also made a4paper a default option

The printers here use US letter paper, so this would be awkward. Please 
default to the size given by ps-paper-type if it is non-nil. On my 
desktop, ps-paper-type is the symbol 'letter'. You can use 
ps-page-dimensions-database to convert symbols like 'letter' to 
PostScript points.

Also, please follow the guidelines in CONTRIBUTE for commit messages. 
Among other things the first line should be brief (50 chars, say) and 
the second line empty.

And thanks for improving this part of Emacs.

Re: cal-tex.el landscape patch

[Glenn Morris]

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.

RE: cal-tex.el landscape patch

[Vincent Belaïche]

Hello,

Comments below,


De : Glenn Morris <rgm@gnu.org>
Envoyé : lundi 28 août 2017 19:55
À : Vincent Belaïche
Cc : Edward Reingold; emacs-devel
Objet : Re: cal-tex.el landscape patch
>
>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.

So you mean that I should remove the argument parsing. But it cannot be
removed all together because anyway I parse this argument to detect
landscape option.

>And please don't default to A4 paper, since the default locale for
>Emacs is the US one (spelling etc).

Would it be acceptable then if letterpaper is specified. Specifying the
paper is a good idea because some latex distribution can be installed
with a default paper size different from letterpaper.

If we want \paperwidth and \paperheight defined to the correct value in
the document so that the LaTeX code compute the cell size autonmously
and the users can configure what paper size they want, then it is good
that paper is explicitely defined.

>
>> +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?

If nil, then the options list is the default one, that is in the case of
my path `12pt,a4paper'. It could make that an empty string generates
`\documentclass{article}' instead of `\documentclass[]{article}' that
would remove the need of having the `t' case.

I thought that it was good to let the function caller decide what they
want in the output, but since `\documentclass{article}' and
`\documentclass[]{article}' are equivalent this is not really usefull.


>
>>    (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]")

Reading the patch, I realize that I did a mistake, it should have been

   ((null args) "[12pt,a4paper]")

Or with whatever paper size you want a default.


>> 
>> + (t ""))))
>
>This seems overly complicated to me.

I see here the main point for acceptance of this patch. Would it be more
acceptabe if the (string-match blah blah blah) would be replaced by some
defsubst like

(cal-tex-has-size-p args)

(cal-tex-has-paper-p args)



>Again, it was a bad suggestion of mine to have default values.

As I wrote, having the paper size explicitely configured would help if
different paper sizes are to be supported in the future. And this
feature is part of the TO DO list at the beginning of cal-tex.el :

;; TO DO

[...]

;;     (*)  Make calendar styles for A4 paper.

Now, this paper and point size defaults should certainly be some
defcustom. Even more, there should be one first defcustom, call it
cal-tex-default-options for all the types of calendars, and one other
defcustom, call it cal-tex-default-options-alist, for types calendar
that need some particular configuration different from
cal-tex-default-options.

>string-match-p would avoid the need to save-match-data.

OK, noted. I will use this in the future.

  Vincent.

RE: cal-tex.el landscape patch

[Vincent Belaïche]

[-- Attachment #1: Type: text/plain, Size: 5420 bytes --]

Hello Glenn,
Maybe I have missed your answer. If there was no answer yet, please take your time, no hurry, I just wanted to make sure that I have not overlooked anything.
  V.

________________________________
De : Vincent Belaïche <vincent.b.1@hotmail.fr>
Envoyé : mercredi 30 août 2017 11:45
À : Glenn Morris
Cc : Edward Reingold; emacs-devel
Objet : RE: cal-tex.el landscape patch


Hello,

Comments below,


De : Glenn Morris <rgm@gnu.org>
Envoyé : lundi 28 août 2017 19:55
À : Vincent Belaïche
Cc : Edward Reingold; emacs-devel
Objet : Re: cal-tex.el landscape patch
>
>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.

So you mean that I should remove the argument parsing. But it cannot be
removed all together because anyway I parse this argument to detect
landscape option.

>And please don't default to A4 paper, since the default locale for
>Emacs is the US one (spelling etc).

Would it be acceptable then if letterpaper is specified. Specifying the
paper is a good idea because some latex distribution can be installed
with a default paper size different from letterpaper.

If we want \paperwidth and \paperheight defined to the correct value in
the document so that the LaTeX code compute the cell size autonmously
and the users can configure what paper size they want, then it is good
that paper is explicitely defined.

>
>> +Please note that if ARGS is \"\" then
>> +\"\\documentclass[]{article}\<file://\\documentclass[]{article}\>" is inserted, while if ARGS it `t'
>> +then \"\\documentclass{article}\<file://\\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?

If nil, then the options list is the default one, that is in the case of
my path `12pt,a4paper'. It could make that an empty string generates
`\documentclass{article}' instead of `\documentclass[]{article}' that
would remove the need of having the `t' case.

I thought that it was good to let the function caller decide what they
want in the output, but since `\documentclass{article}' and
`\documentclass[]{article}' are equivalent this is not really usefull.


>
>>    (set-buffer (generate-new-buffer cal-tex-buffer))
>> -  (insert (format "\\documentclass%s{article}\n<file://\\documentclass%s{article}\n>"
>> -                  (if (stringp args)
>> -                      (format "[%s]" args)
>> -                    "")))
>> +  (save-match-data
>> +    (insert (format "\\documentclass%s{article}\n<file://\\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]")

Reading the patch, I realize that I did a mistake, it should have been

   ((null args) "[12pt,a4paper]")

Or with whatever paper size you want a default.


>>
>> + (t ""))))
>
>This seems overly complicated to me.

I see here the main point for acceptance of this patch. Would it be more
acceptabe if the (string-match blah blah blah) would be replaced by some
defsubst like

(cal-tex-has-size-p args)

(cal-tex-has-paper-p args)



>Again, it was a bad suggestion of mine to have default values.

As I wrote, having the paper size explicitely configured would help if
different paper sizes are to be supported in the future. And this
feature is part of the TO DO list at the beginning of cal-tex.el :

;; TO DO

[...]

;;     (*)  Make calendar styles for A4 paper.

Now, this paper and point size defaults should certainly be some
defcustom. Even more, there should be one first defcustom, call it
cal-tex-default-options for all the types of calendars, and one other
defcustom, call it cal-tex-default-options-alist, for types calendar
that need some particular configuration different from
cal-tex-default-options.

>string-match-p would avoid the need to save-match-data.

OK, noted. I will use this in the future.

  Vincent.

[-- Attachment #2: Type: text/html, Size: 9057 bytes --]

Re: cal-tex.el landscape patch

[Glenn Morris]

Hi, I think you should go ahead and do whatever you think is best.
Thanks.