* Re: [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage [not found] ` <20231031225840.6A0FEC06553@vcs2.savannah.gnu.org> @ 2023-11-01 3:05 ` Stefan Monnier 2023-11-01 6:07 ` Karl Fogel 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2023-11-01 3:05 UTC (permalink / raw) To: emacs-devel; +Cc: Karl Fogel > This change does not affect the behavior of `org-table-make-reference' > because `eq' treats all instances of the empty string as the same > object anyway, e.g., `(eq (string-trim "aaabbb" "a+" "b+") "")' ==> t. Not quite so: (eq (string-to-multibyte "") "") => nil -- Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage 2023-11-01 3:05 ` [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage Stefan Monnier @ 2023-11-01 6:07 ` Karl Fogel 2023-11-01 9:04 ` Bastien 0 siblings, 1 reply; 5+ messages in thread From: Karl Fogel @ 2023-11-01 6:07 UTC (permalink / raw) To: Emacs Devel; +Cc: Stefan Monnier [-- Attachment #1: Type: text/plain, Size: 1507 bytes --] Stefan Monnier wrote: >> This change does not affect the behavior of >> `org-table-make-reference' because `eq' treats all instances >> of the empty string as the same object anyway, e.g., >> `(eq (string-trim "aaabbb" "a+" "b+") "")' ==> t. > >Not quite so: (eq (string-to-multibyte "") "") => nil Thank you for noticing this, Stefan. I did many tests with `eq' and various kinds of generated empty strings before writing that commit message -- but, alas, I didn't think of testing with a multibyte string. (I also looked in the documentation, and the fact that there was no explicit mention of all empty strings being the same object should have tipped me off!) So, I believe this means there is an error in that commit message. I've attached the commitinfo below for easy reference, with 'inline' disposition. However, the code change itself remains correct... I think? That is: the previous code presumably had a latent bug, in that there *could* have been times when the `eq' test would fail when comparing against a multibyte empty string. I don't know much about how Org Mode gets merged into Emacs, nor whether there are opportunities for rebasing anywhere along the way in that process. If there's a way to update that commit message, then I'd do so (I can post a new one here). But if there's no such opportunity, then so be it: I'll be on the permanent record with a technical mistake in a log message. I'm sure it wouldn't be the first time :-/. Best regards, -Karl [-- Attachment #2: commit ee0961ba317 --] [-- Type: text/plain, Size: 878 bytes --] commit ee0961ba3170f7bc89c2f6fabda4b6ea2e7a2c88 Author: Karl Fogel <kfogel@red-bean.com> AuthorDate: Mon Oct 30 10:33:29 2023 -0500 Commit: Bastien Guerry <bzg@gnu.org> CommitDate: Tue Oct 31 21:38:23 2023 +0100 lisp/org-table.el: fix warning about `eq' usage * lisp/org-table.el (org-table-make-reference): Use `equal' instead of `eq' to compare strings. This change makes the following warning go away: Warning (comp): org-table.el:2867:23: \ Warning: `eq' called with literal string that may never match (arg 2) This change does not affect the behavior of `org-table-make-reference' because `eq' treats all instances of the empty string as the same object anyway, e.g., `(eq (string-trim "aaabbb" "a+" "b+") "")' ==> t. The only effect of this change is to eliminate the warning. M lisp/org-table.el ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage 2023-11-01 6:07 ` Karl Fogel @ 2023-11-01 9:04 ` Bastien 2023-11-01 11:31 ` Stefan Monnier 0 siblings, 1 reply; 5+ messages in thread From: Bastien @ 2023-11-01 9:04 UTC (permalink / raw) To: Karl Fogel; +Cc: Emacs Devel, Stefan Monnier Hi Karl, Karl Fogel <kfogel@red-bean.com> writes: > I don't know much about how Org Mode gets merged into Emacs, nor > whether there are opportunities for rebasing anywhere along the way in > that process. If there's a way to update that commit message, then > I'd do so (I can post a new one here). But if there's no such > opportunity, then so be it: I'll be on the permanent record with a > technical mistake in a log message. I'm sure it wouldn't be the first > time :-/. So be it! It isn't the first time for many of us :) Thanks again for the fix, best, -- Bastien ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage 2023-11-01 9:04 ` Bastien @ 2023-11-01 11:31 ` Stefan Monnier 2023-11-01 16:22 ` Karl Fogel 0 siblings, 1 reply; 5+ messages in thread From: Stefan Monnier @ 2023-11-01 11:31 UTC (permalink / raw) To: Bastien; +Cc: Karl Fogel, Emacs Devel > So be it! It isn't the first time for many of us :) I'm not worried about such problems, indeed. My message was only meant to spread awareness. Stefan ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage 2023-11-01 11:31 ` Stefan Monnier @ 2023-11-01 16:22 ` Karl Fogel 0 siblings, 0 replies; 5+ messages in thread From: Karl Fogel @ 2023-11-01 16:22 UTC (permalink / raw) To: Stefan Monnier; +Cc: Bastien, Emacs Devel On 01 Nov 2023, Stefan Monnier wrote: >> So be it! It isn't the first time for many of us :) > >I'm not worried about such problems, indeed. >My message was only meant to spread awareness. Well, thanks for doing so -- I was glad to learn. Best regards, -Karl ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-11-01 16:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <169879312007.20093.7465071871654518215@vcs2.savannah.gnu.org> [not found] ` <20231031225840.6A0FEC06553@vcs2.savannah.gnu.org> 2023-11-01 3:05 ` [elpa] externals/org ee0961ba31: lisp/org-table.el: fix warning about `eq' usage Stefan Monnier 2023-11-01 6:07 ` Karl Fogel 2023-11-01 9:04 ` Bastien 2023-11-01 11:31 ` Stefan Monnier 2023-11-01 16:22 ` Karl Fogel
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).