Re: [PATCH] Fix a possibly problematic string comparison

[Ihor Radchenko]

Rudolf Adamkovič  writes:

> * lisp/org-table.el (org-table-make-reference): Replace 'eq' with
> 'string-empty-p' to resolve "Warning: 'eq' called with literal string
> that may never match" issued on every 'make' invocation.

Handled, on main.
https://git.savannah.gnu.org/cgit/emacs/org-mode.git/commit/?id=bd5665e01
I went with the non-breaking change.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] Fix a possibly problematic string comparison

[Rudolf Adamkovič]

Thank you for the explanation!

Given the complexity of the caller, that is
the `org-table-eval-formula' function, as
well as, the fact that I have yet to learn
about the spreadsheet functionality, I am
backing away from this bugfix.

Sorry for the noise.

Rudy
-- 
"Programming reliably -- must be an activity
of an undeniably mathematical nature […] You
see, mathematics is about thinking, and doing
mathematics is always trying to think as well
as possible."  -- Edsger W. Dijkstra, 1981

Rudolf Adamkovič  [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

Re: [PATCH] Fix a possibly problematic string comparison

[Ihor Radchenko]

Rudolf Adamkovič  writes:

> Ihor Radchenko  writes:
>
>> This is not as trivial. Applying this patch will break tests.
>> One needs to carefully examine the org-table logic to fix this
>> particular warning.
>
> Wow!  You are right.  How is that even possible?  In other words, what
> value (of 'elements') can possibly be object-equal to a literal "" but
> not string-equal to the same literal "" or vice versa?

This is not what is happening. 

-   (if (and (eq elements "") (not keep-empty))

(eq elements "") is always nil - the warning is right. We never enter
this branch of `if'.

+   (if (and (string-empty-p elements) (not keep-empty))

After the patch, we do enter this branch of `if', but it breaks tests.

So, it is not just an innocent warning - it reveals a fault in logic
elsewhere in the code.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

Re: [PATCH] Fix a possibly problematic string comparison

[Rudolf Adamkovič]

Ihor Radchenko  writes:

> This is not as trivial. Applying this patch will break tests.
> One needs to carefully examine the org-table logic to fix this
> particular warning.

Wow!  You are right.  How is that even possible?  In other words, what
value (of 'elements') can possibly be object-equal to a literal "" but
not string-equal to the same literal "" or vice versa?

Rudy
-- 
"Logic is a science of the necessary laws of thought, without which no
employment of the understanding and the reason takes place."
-- Immanuel Kant, 1785

Rudolf Adamkovič  [he/him]
Studenohorská 25
84103 Bratislava
Slovakia

Re: [PATCH] Fix a possibly problematic string comparison

[Ihor Radchenko]

Rudolf Adamkovič  writes:

> * lisp/org-table.el (org-table-make-reference): Replace 'eq' with
> 'string-empty-p' to resolve "Warning: 'eq' called with literal string
> that may never match" issued on every 'make' invocation.
>
> - (if (and (eq elements "") (not keep-empty))
> + (if (and (string-empty-p elements) (not keep-empty))

This is not as trivial. Applying this patch will break tests.
One needs to carefully examine the org-table logic to fix this
particular warning.

-- 
Ihor Radchenko // yantar92,
Org mode contributor,
Learn more about Org mode at .
Support Org development at ,
or support my work at 

[PATCH] Fix a possibly problematic string comparison

[Rudolf Adamkovič]

* lisp/org-table.el (org-table-make-reference): Replace 'eq' with
'string-empty-p' to resolve "Warning: 'eq' called with literal string
that may never match" issued on every 'make' invocation.
---
 lisp/org-table.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org-table.el b/lisp/org-table.el
index f5a433c7d..b82749469 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -2864,7 +2864,7 @@ list, `literal' is for the format specifier L."
   (if lispp
  (if (eq lispp 'literal)
  elements
-   (if (and (eq elements "") (not keep-empty))
+   (if (and (string-empty-p elements) (not keep-empty))
""
  (prin1-to-string
   (if numbers (string-to-number elements) elements
-- 
2.41.0