Patch for a simple nil check in `calc-graph-add-curve'

Patch for a simple nil check in `calc-graph-add-curve'

[Narendra Joshi]

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

Hi,

Can someone please add this simple `nil' check in the attached patch?

Best,
Narendra

[-- Attachment #2: 0001-Add-nil-check-for-pstyle-and-lstyle-in-calc-graph-ad.patch --]
[-- Type: text/x-patch, Size: 1134 bytes --]

From e6132b54351ecb487001cc4592f313efcb3d8f15 Mon Sep 17 00:00:00 2001
From: Narendra Joshi <narendraj9@gmail.com>
Date: Sat, 21 Mar 2020 20:45:38 +0100
Subject: [PATCH] Add nil check for pstyle and lstyle in calc-graph-add-curve.

---
 lisp/calc/calc-graph.el | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lisp/calc/calc-graph.el b/lisp/calc/calc-graph.el
index 4cdfdbd4b9..4feddd9e4d 100644
--- a/lisp/calc/calc-graph.el
+++ b/lisp/calc/calc-graph.el
@@ -211,9 +211,9 @@ calc-graph-add-curve
       (setq pstyle (and (eq (car-safe pstyle) 'vec) (nth (1+ num) pstyle)))
       (setq lstyle (and (eq (car-safe lstyle) 'vec) (nth (1+ num) lstyle))))
     (calc-graph-set-styles
-     (or (and (Math-num-integerp lstyle) (math-trunc lstyle))
+     (or (and lstyle (Math-num-integerp lstyle) (math-trunc lstyle))
          0)
-     (or (and (Math-num-integerp pstyle) (math-trunc pstyle))
+     (or (and pstyle (Math-num-integerp pstyle) (math-trunc pstyle))
          (if (eq (car-safe (calc-var-value (nth 2 ydata))) 'vec)
              0 -1))
      (math-contains-sdev-p (eval (nth 2 ydata))))))
-- 
2.20.1

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Eli Zaretskii]

> From: Narendra Joshi <narendraj9@gmail.com>
> Date: Sat, 21 Mar 2020 20:48:52 +0100
> 
> Can someone please add this simple `nil' check in the attached patch?

Thanks, but could you please provide the rationale?

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Narendra Joshi]

A simple `calc-graph-fast' call with the following values on Emacs
Calc stack fails:

3:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
2:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
1:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
<top of the stack>

It fails because `calc-graph-fast` eventually calls `math-trunc` on
the line and point style values which aren't set yet. Since they are
`nil` we should avoid calling `math-trunc` on them and take a default
value. The patch just adds a `nil` check for these variables `lstyles`
(LineStyles) and `pstyles` (PointStyles).

 - Narendra

On Sat, Mar 21, 2020 at 9:27 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: Narendra Joshi <narendraj9@gmail.com>
> > Date: Sat, 21 Mar 2020 20:48:52 +0100
> >
> > Can someone please add this simple `nil' check in the attached patch?
>
> Thanks, but could you please provide the rationale?

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Bruno Félix Rezende Ribeiro]

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

Hello,

FWIW, this has /not/ been integrated into master yet.

Narendra Joshi <narendraj9@gmail.com> writes:

> A simple `calc-graph-fast' call with the following values on Emacs
> Calc stack fails:
> 3:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
> 2:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
> 1:  [1, 2, 3, 4, 5, 6, 7, 8, 9, 10]
> <top of the stack>
> It fails because `calc-graph-fast` eventually calls `math-trunc` on
> the line and point style values which aren't set yet. Since they are
> `nil` we should avoid calling `math-trunc` on them and take a default
> value. The patch just adds a `nil` check for these variables `lstyles`
> (LineStyles) and `pstyles` (PointStyles).
>  - Narendra
> On Sat, Mar 21, 2020 at 9:27 PM Eli Zaretskii <eliz@gnu.org> wrote:
>> > From: Narendra Joshi <narendraj9@gmail.com>
>> > Date: Sat, 21 Mar 2020 20:48:52 +0100
>> > Can someone please add this simple `nil' check in the attached patch?
>> Thanks, but could you please provide the rationale?

-- 
Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
<http://oitofelix.freeshell.org/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 454 bytes --]

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Eli Zaretskii]

> From: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>
> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
> Date: Wed, 08 Apr 2020 11:47:53 -0300
> 
> FWIW, this has /not/ been integrated into master yet.

It hasn't?  I though that was bug#40155, already fixed.

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Narendra Joshi]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
>> Date: Wed, 08 Apr 2020 11:47:53 -0300
>> 
>> FWIW, this has /not/ been integrated into master yet.
>
> It hasn't?  I though that was bug#40155, already fixed.

The issue that I reported was fixed with a different patch. After 
the
move to big integers, `Math-integer-p` needed to return false for 
`nil'
values. Fixed with commit: 
c2b8ce4439935e2e158d4357d234135a251c5767. 

Best,
-- 
Narendra Joshi

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Bruno Félix Rezende Ribeiro]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>
>> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
>> Date: Wed, 08 Apr 2020 11:47:53 -0300
>> FWIW, this has /not/ been integrated into master yet.
> It hasn't?  I though that was bug#40155, already fixed.

Sorry for the noise, then.  Clearly my heuristics for deciding if
something has been integrated into master is not good enough.  I have to
put GNU debbugs in the loop.  Any other general hints on how to decide
whether some report has been addressed?

-- 
Bruno Félix Rezende Ribeiro (oitofelix) [0x28D618AF]
<http://oitofelix.freeshell.org/>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 487 bytes --]

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Robert Pluim]

>>>>> On Fri, 10 Apr 2020 11:07:00 -0300, Bruno Félix Rezende Ribeiro <oitofelix@gnu.org> said:

    Bruno> Eli Zaretskii <eliz@gnu.org> writes:
    >>> From: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>
    >>> Cc: Eli Zaretskii <eliz@gnu.org>,  emacs-devel@gnu.org
    >>> Date: Wed, 08 Apr 2020 11:47:53 -0300
    >>> FWIW, this has /not/ been integrated into master yet.
    >> It hasn't?  I though that was bug#40155, already fixed.

    Bruno> Sorry for the noise, then.  Clearly my heuristics for deciding if
    Bruno> something has been integrated into master is not good enough.  I have to
    Bruno> put GNU debbugs in the loop.  Any other general hints on how to decide
    Bruno> whether some report has been addressed?

When fixing a bug, the bug number should be in the commit
message. There should be no need to involve debbugs unless multiple
reports have been merged (and perhaps in that case the commit message
should mention all of them, although thatʼs not something Iʼve been
doing).

git log --grep [Bb]ug#40155

=>
    commit c2b8ce4439935e2e158d4357d234135a251c5767
    Author: Mattias Engdegård <mattiase@acm.org>
    Date:   Fri Mar 27 18:11:18 2020 +0100

        Calc: don't treat nil as an integer (bug#40155)

Robert

Re: Patch for a simple nil check in `calc-graph-add-curve'

[Eli Zaretskii]

> From: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>
> Cc: Bruno Félix Rezende Ribeiro <oitofelix@gnu.org>,
>   narendraj9@gmail.com,  emacs-devel@gnu.org
> Date: Fri, 10 Apr 2020 11:07:00 -0300
> 
> >> FWIW, this has /not/ been integrated into master yet.
> > It hasn't?  I though that was bug#40155, already fixed.
> 
> Sorry for the noise, then.  Clearly my heuristics for deciding if
> something has been integrated into master is not good enough.  I have to
> put GNU debbugs in the loop.  Any other general hints on how to decide
> whether some report has been addressed?

I can only suggest reading the traffic both here and on bug-gnu-emacs.