bug#40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

bug#40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Narendra Joshi]

+ Start Emacs Calc with `C-x * *`.
+ Enter the following formula with:
  ' (a + b)^3 / c d
+ Move point to be before "+" and use "j s" to select the 
sub-formula "(a + b)".
+ This should have hidden the rest of the formula. But it does 
not.

           3
    (a + b)
1*  --------
      c d

Selecting a Calc sub-formula doesn't highlight it. For example, in 
the stack entry above, I have selected (a + b) but other parts of 
the formula are still visible. All commands operate on the 
selected formula and it is selected as evident from the '*' after 
the number of the stack entry. It is not highlighted. This seems 
to be working fine in Emacs 26. It's broken in the latest Emacs 
versions: Emacs 27 and Emacs 28. I can confirm that it is not 
happening because of any of my configuration because I tried the 
same thing with `emacs -Q` and the result was the same.

Best, 
-- 
Narendra Joshi

bug#40185: #40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Mattias Engdegård]

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

Thanks for the report. Does this patch help?


[-- Attachment #2: calc-selection.patch --]
[-- Type: application/octet-stream, Size: 343 bytes --]

diff --git a/lisp/calc/calc.el b/lisp/calc/calc.el
index 4b2a938a5f..4e4fb67173 100644
--- a/lisp/calc/calc.el
+++ b/lisp/calc/calc.el
@@ -2927,6 +2927,7 @@ math-div-float
 
 
 
+(defvar math-comp-selected)
 (defvar calc-selection-cache-entry)
 ;;; Format the number A as a string.  [X N; X Z] [Public]
 (defun math-format-stack-value (entry)

bug#40185: #40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Narendra Joshi]

Mattias Engdegård <mattiase@acm.org> writes:

Thanks for the quick fix. It seems to be working. Can you please 
help me
understand what exactly the issue was and how declaring the 
variable
fixed it? I see that it is `defvar`ed  in `calc-ext.el`.

> Thanks for the report. Does this patch help?
>
> diff --git a/lisp/calc/calc.el b/lisp/calc/calc.el
> index 4b2a938a5f..4e4fb67173 100644
> --- a/lisp/calc/calc.el
> +++ b/lisp/calc/calc.el
> @@ -2927,6 +2927,7 @@ math-div-float
>  
>  
>  
> +(defvar math-comp-selected)
>  (defvar calc-selection-cache-entry)
>  ;;; Format the number A as a string.  [X N; X Z] [Public]
>  (defun math-format-stack-value (entry)
>

-- 
Narendra Joshi

bug#40185: #40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Mattias Engdegård]

23 mars 2020 kl. 20.06 skrev Narendra Joshi <narendraj9@gmail.com>:

> Can you please help me
> understand what exactly the issue was and how declaring the variable
> fixed it? I see that it is `defvar`ed  in `calc-ext.el`.

'defvar' without an initial value is a declaration scoped to the surrounding lexical scope (or file, if at top level). It appears that calc.el does not require calc-ext, so that math-comp-selected was assumed to be lexical where it was bound in math-format-stack-value. This caused it to have the wrong number further down the call tree (in math-comp-to-string-flat, I think).

In Emacs 26 and earlier, calc.el used dynamic binding.

Thanks for verifying! Patch pushed to emacs-27.

bug#40185: #40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Narendra Joshi]

Thanks for the explanation. :) I think I got it. It seems to me that
porting a large package like `calc` to use lexical binding everywhere
would mean fixing a lot of such bugs.

Is the fix also going to be merged to `master` or will it be merged
sometime later?

- Narendra

On Mon, Mar 23, 2020 at 9:37 PM Mattias Engdegård <mattiase@acm.org> wrote:
>
> 23 mars 2020 kl. 20.06 skrev Narendra Joshi <narendraj9@gmail.com>:
>
> > Can you please help me
> > understand what exactly the issue was and how declaring the variable
> > fixed it? I see that it is `defvar`ed  in `calc-ext.el`.
>
> 'defvar' without an initial value is a declaration scoped to the surrounding lexical scope (or file, if at top level). It appears that calc.el does not require calc-ext, so that math-comp-selected was assumed to be lexical where it was bound in math-format-stack-value. This caused it to have the wrong number further down the call tree (in math-comp-to-string-flat, I think).
>
> In Emacs 26 and earlier, calc.el used dynamic binding.
>
> Thanks for verifying! Patch pushed to emacs-27.
>

bug#40185: #40185: 28.0.50; Emacs Calc selection of a sub-formula doesn't highlight it

[Mattias Engdegård]

23 mars 2020 kl. 23.37 skrev Narendra Joshi <narendraj9@gmail.com>:

> Thanks for the explanation. :) I think I got it. It seems to me that
> porting a large package like `calc` to use lexical binding everywhere
> would mean fixing a lot of such bugs.

Yes, currently only a few files have been converted to lexical binding, and the Calc code relies heavily on dynamic variables everywhere. The code also uses side-effects a lot. Rewriting it in a more modern, functional style would do wonders for maintainability and correctness, but its size and complexity is likely to make it quite an enterprise.

> Is the fix also going to be merged to `master` or will it be merged
> sometime later?

It will be merged in a few days. If not, please send a reminder.