Please check-in Belanger's Calc patches

Please check-in Belanger's Calc patches

[Tacvek]

Perhaps the emacs developers are slow, but i find it unreasonable that a bug
with a patch approved by a compenents maintainer has been waiting 3/4 of a
year before getting checked in. Yet this is the case with Belanger's Calc
patches. they were approved by the Calc maintainer
(http://lists.gnu.org/archive/html/emacs-devel/2003-10/msg00719.html), and
posted to the list twice, the second time by an indirect request from RMS
himself
(http://lists.gnu.org/archive/html/emacs-devel/2004-02/msg00437.html).

I believe the patches may now be slightly out of date, so can you remake
them against the current cvs, Belanger?

I ask that this problem be fixed, and included in the next minor release.

Re: Please check-in Belanger's Calc patches

[Karl Fogel]

"Tacvek" <unknown_kev_cat@hotmail.com> writes:
> Perhaps the emacs developers are slow, but i find it unreasonable that a bug
> with a patch approved by a compenents maintainer has been waiting 3/4 of a
> year before getting checked in. Yet this is the case with Belanger's Calc
> patches. they were approved by the Calc maintainer
> (http://lists.gnu.org/archive/html/emacs-devel/2003-10/msg00719.html), and
> posted to the list twice, the second time by an indirect request from RMS
> himself
> (http://lists.gnu.org/archive/html/emacs-devel/2004-02/msg00437.html).
> 
> I believe the patches may now be slightly out of date, so can you remake
> them against the current cvs, Belanger?
> 
> I ask that this problem be fixed, and included in the next minor release.

I might be missing some background here, but is there some reason the
Calc maintainer can't commit these changes?  When patches are approved
by the component maintainer, then normally the maintainer just checks
them in.

IOW, why is this situation special?

Thanks,
-Karl

Re: Please check-in Belanger's Calc patches

[Jay Belanger]

"Tacvek" <unknown_kev_cat@hotmail.com> writes:
...
> I believe the patches may now be slightly out of date, so can you remake
> them against the current cvs, Belanger?

The same patches should work, since there haven't really been any
changes to calc since I made them.  At any rate, I've been using the
same patches with the current cvs.  

> I ask that this problem be fixed, and included in the next minor release.

Thanks; I was just about to re-bring this up myself.

Jay

Re: Please check-in Belanger's Calc patches

[Richard Stallman]

We're all volunteers, and none of us understands Calc.  Calc used to
have its own maintainer, but I seem to recall he stepped down and
there isn't one any more.  I will double-check that now.

Re: Please check-in Belanger's Calc patches

[Karl Fogel]

Richard Stallman <rms@gnu.org> writes:
> We're all volunteers, and none of us understands Calc.  Calc used to
> have its own maintainer, but I seem to recall he stepped down and
> there isn't one any more.  I will double-check that now.

I see, thanks.

If the maintainer is not available, I'll take a look at this (I don't
know Calc either, but probably can learn enough to apply this patch).

-Karl

Re: Please check-in Belanger's Calc patches

[Karl Fogel]

I wrote:
> If the maintainer is not available, I'll take a look at this (I don't
> know Calc either, but probably can learn enough to apply this patch).

Jay, I took a look at your patch, and tested that it fixes the bug.  I
don't know how on earth you deduced the nature of the hitherto
undefined variable `calc-mode-var-list' just from looking at the code,
but nice job :-).

The change is more than 10 lines, so the FSF needs to get copyright
assignment papers from you.  I trust that's okay?  Somewhat to my
surprise, the assignment forms do not appear to be online, so I'm
going through the procedures described in

   http://www.gnu.org/prep/maintain_5.html#SEC5

to get you the forms.

In the meantime, could you check over the following ChangeLog entry?
I'm no expert in Calc, so might have failed to mention something
important about the change.  Also, if you wanted to write a doc string
for the variable `calc-mode-var-list', that would be great (though
certainly not necessary for checking in this change).

Thanks,
-Karl

--------------------8-<-------cut-here---------8-<-----------------------

2004-07-20  Karl Fogel  <kfogel@red-bean.com>

	Apply Jay Belanger <belanger@truman.edu>'s patch from
	http://lists.gnu.org/archive/html/emacs-devel/2003-10/msg00719.html.
	This fixes the bug described in
	http://lists.gnu.org/archive/html/emacs-devel/2004-02/msg00437.html,
        and was apparently approved by Calc's maintainer.  See
	http://lists.gnu.org/archive/html/emacs-devel/2004-07/msg00316.html
	and the thread hanging therefrom for more information.
	
	* calc/calc.el (calc-mode-var-list): Define this variable.
	(calc-always-load-extensions, calc-line-numbering,
	calc-line-breaking, calc-display-just, calc-display-origin,
	calc-number-radix, calc-leading-zeros, calc-group-digits,
	calc-group-char, calc-point-char, calc-frac-format,
	calc-prefer-frac, calc-hms-format, calc-date-format,
	calc-float-format, calc-full-float-format, calc-complex-format,
	calc-complex-mode, calc-infinite-mode, calc-display-strings,
	calc-matrix-just, calc-break-vectors, calc-full-vectors,
	calc-full-trail-vectors, calc-vector-commas, calc-vector-brackets,
	calc-matrix-brackets, calc-language, calc-language-option,
	calc-left-label, calc-right-label, calc-word-size,
	calc-previous-modulo, calc-simplify-mode, calc-auto-recompute,
	calc-display-raw, calc-internal-prec, calc-angle-mode,
	calc-algebraic-mode, calc-incomplete-algebraic-mode,
	calc-symbolic-mode, calc-matrix-mode, calc-shift-prefix,
	calc-window-height, calc-display-trail, calc-show-selections,
	calc-use-selections, calc-assoc-selections,
	calc-display-working-message, calc-auto-why , calc-timing,
	calc-mode-save-mode, calc-standard-date-formats,
	calc-autorange-units, calc-was-keypad-mode, calc-full-mode,
	calc-user-parse-tables, calc-gnuplot-default-device,
	calc-gnuplot-default-output, calc-gnuplot-print-device,
	calc-gnuplot-print-output, calc-gnuplot-geometry,
	calc-graph-default-resolution, calc-graph-default-resolution-3d,
	calc-invocation-macro, calc-show-banner): Define these as part of
          `calc-mode-var-list's initialization, instead of using defvar.

	(calc-inverse-flag, calc-hyperbolic-flag, calc-keep-args-flag):
        Give doc strings to these variables.
	
	* calc/calc-embed.el (calc-embedded-set-modes): Use
	`calc-mode-var-list' correctly.

Re: Please check-in Belanger's Calc patches

[Stefan]

[...]
> 	calc-invocation-macro, calc-show-banner): Define these as part of
>           `calc-mode-var-list's initialization, instead of using defvar.

Not defining those vars with defvar means that the byte-compiler doesn't
get the info it wants.  It doesn't necessarily lead to wrong code, but it's
very bad coding style and can prevent optimizations (which are admittedly
not currently implemented).  It also leads to lots and lots of spurious
warnings during byte-compilation.


        Stefan

Re: Please check-in Belanger's Calc patches

[Jay Belanger]

Stefan <monnier@iro.umontreal.ca> writes:

> [...]
>> 	calc-invocation-macro, calc-show-banner): Define these as part of
>>           `calc-mode-var-list's initialization, instead of using defvar.
>
> Not defining those vars with defvar means that the byte-compiler doesn't
> get the info it wants.  It doesn't necessarily lead to wrong code, but it's
> very bad coding style and can prevent optimizations (which are admittedly
> not currently implemented).  It also leads to lots and lots of spurious
> warnings during byte-compilation.

Calc stores those variables in calc-mode-var-list so that functions
(which store and reset those variables) can easily access them.  
There are about four functions which do this.  If these variables are
set individually by defvar, then the store-reset functions would have
to be redone.  Restoring calc-mode-var-list is easier, but redoing the
functions is doable, no doubt.

Jay

Re: Please check-in Belanger's Calc patches

[Stefan Monnier]

>> [...]
>>> calc-invocation-macro, calc-show-banner): Define these as part of
>>> `calc-mode-var-list's initialization, instead of using defvar.
>> 
>> Not defining those vars with defvar means that the byte-compiler doesn't
>> get the info it wants.  It doesn't necessarily lead to wrong code, but it's
>> very bad coding style and can prevent optimizations (which are admittedly
>> not currently implemented).  It also leads to lots and lots of spurious
>> warnings during byte-compilation.

> Calc stores those variables in calc-mode-var-list so that functions
> (which store and reset those variables) can easily access them.  
> There are about four functions which do this.  If these variables are
> set individually by defvar, then the store-reset functions would have
> to be redone.  Restoring calc-mode-var-list is easier, but redoing the
> functions is doable, no doubt.

But as a short-term fix, can't they be both defvar'd and listed in
calc-mode-var-list?


        Stefan

Re: Please check-in Belanger's Calc patches

[Jay Belanger]

Stefan Monnier <monnier@iro.umontreal.ca> writes:
...
>> Calc stores those variables in calc-mode-var-list so that functions
>> (which store and reset those variables) can easily access them.
>> There are about four functions which do this.  If these variables are
>> set individually by defvar, then the store-reset functions would have
>> to be redone.  Restoring calc-mode-var-list is easier, but redoing the
>> functions is doable, no doubt.
>
> But as a short-term fix, can't they be both defvar'd and listed in
> calc-mode-var-list?

Use calc-mode-var-list to store and set their values after they've
been defvar'd?  In that case, why would it only be a short term fix?

Jay

Re: Please check-in Belanger's Calc patches

[Jay Belanger]

Jay Belanger <belanger@truman.edu> writes:
...
> Use calc-mode-var-list to store and set their values after they've
> been defvar'd?  In that case, why would it only be a short term fix?

As a partial answer to my question, I suppose that a lot of the
usefulness of calc-mode-var-list could be moved to defcustom.

Jay