all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#64205: Fix missing border cell when using orgtbl-to-table.el function
@ 2023-06-20 21:57 Jakub Ječmínek
  2023-06-21 12:45 ` Robert Pluim
  2023-06-21 14:08 ` bug#64205: Fix missing cell border " Jakub Ječmínek
  0 siblings, 2 replies; 8+ messages in thread
From: Jakub Ječmínek @ 2023-06-20 21:57 UTC (permalink / raw)
  To: 64205


[-- Attachment #1.1: Type: text/plain, Size: 980 bytes --]

Hello,
I would like to take this opportunity to express my deep appreciation for
the valuable work the Emacs community has contributed to the development
and maintenance of this remarkable editor.

I've found a bug in the `orgtbl-to-table.el' function. When using
`orgtbl-to-table.el' during `org-table-export', last border cell ("|") was
omitted in the output.

Steps to reproduce:

(require 'org-table)
(insert "\n" (orgtbl-to-table.el '(("first colum" "second column") hline
("12" "34")) nil))

Previous implementation output:

| first colum | second column |
+--------------+---------------------+
|          12     |            34           < missing border cell

Current output:

| first colum | second column |
+--------------+---------------------+
|          12    |             34          |


In attachment, you'll find the smallest patch you've ever seen.

Thank you for everything you do for my beloved Emacs.

Best, Jakub Ječmínek

[-- Attachment #1.2: Type: text/html, Size: 1401 bytes --]

[-- Attachment #2: 0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch --]
[-- Type: application/octet-stream, Size: 987 bytes --]

From 688bd0104f1e63df8f757de825481a69c4620f81 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Je=C4=8Dm=C3=ADnek?= <jecminek.k@gmail.com>
Date: Tue, 20 Jun 2023 23:30:49 +0200
Subject: [PATCH] Fix orgtbl-to-table.el function to include last cell border

Previously used buffer-size function is (1- (point-max)) which means
that last cell border was omitted.
---
 lisp/org/org-table.el | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
index 42f234790c5..9d3507e34b7 100644
--- a/lisp/org/org-table.el
+++ b/lisp/org/org-table.el
@@ -6134,7 +6134,7 @@ supported."
     (org-table-align)
     (replace-regexp-in-string
      "-|" "-+"
-     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
+     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (point-max))))))
 
 (defun orgtbl-to-unicode (table params)
   "Convert the `orgtbl-mode' TABLE into a table with unicode characters.
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing border cell when using orgtbl-to-table.el function
  2023-06-20 21:57 bug#64205: Fix missing border cell when using orgtbl-to-table.el function Jakub Ječmínek
@ 2023-06-21 12:45 ` Robert Pluim
  2023-06-21 14:08 ` bug#64205: Fix missing cell border " Jakub Ječmínek
  1 sibling, 0 replies; 8+ messages in thread
From: Robert Pluim @ 2023-06-21 12:45 UTC (permalink / raw)
  To: Jakub Ječmínek; +Cc: 64205

>>>>> On Tue, 20 Jun 2023 23:57:35 +0200, Jakub Ječmínek <jecminek.k@gmail.com> said:

    Jakub> Previously used buffer-size function is (1- (point-max)) which means
    Jakub> that last cell border was omitted.
    Jakub> ---
    Jakub>  lisp/org/org-table.el | 2 +-
    Jakub>  1 file changed, 1 insertion(+), 1 deletion(-)

    Jakub> diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
    Jakub> index 42f234790c5..9d3507e34b7 100644
    Jakub> --- a/lisp/org/org-table.el
    Jakub> +++ b/lisp/org/org-table.el
    Jakub> @@ -6134,7 +6134,7 @@ supported."
    Jakub>      (org-table-align)
    Jakub>      (replace-regexp-in-string
    Jakub>       "-|" "-+"
    Jakub> -     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
    Jakub> +     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (point-max))))))

Thatʼs just (buffer-string).

For bonus points, rewrite it to do the replacement in the temp buffer,
and then return (buffer-string), itʼs likely to be much faster (and
will generate less garbage).

Robert
-- 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
  2023-06-20 21:57 bug#64205: Fix missing border cell when using orgtbl-to-table.el function Jakub Ječmínek
  2023-06-21 12:45 ` Robert Pluim
@ 2023-06-21 14:08 ` Jakub Ječmínek
  2023-06-21 15:24   ` Robert Pluim
  1 sibling, 1 reply; 8+ messages in thread
From: Jakub Ječmínek @ 2023-06-21 14:08 UTC (permalink / raw)
  To: 64205


[-- Attachment #1.1: Type: text/plain, Size: 417 bytes --]

Hello Robert, thank you for your response.

I'm sending a new patch in the attachment. I've also instrumented the
function using the `elp' package, here are the results:

Function Name           Call Count  Elapsed Time  Average Time
orgtbl-to-table.el-old  1001        2.6707780000  0.0026681098
orgtbl-to-table.el-new  1001        2.4684200000  0.0024659540

New version is indeed a little bit faster.

Best, Jakub

[-- Attachment #1.2: Type: text/html, Size: 576 bytes --]

[-- Attachment #2: 0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch --]
[-- Type: application/octet-stream, Size: 1245 bytes --]

From dcb7dfb20157390dd4b234c929ce45d058da48d5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Je=C4=8Dm=C3=ADnek?= <jecminek.k@gmail.com>
Date: Wed, 21 Jun 2023 15:50:31 +0200
Subject: [PATCH] Fix orgtbl-to-table.el function to include last cell border

* lisp/org/org-table.el (orgtbl-to-table.el): Perform character
replacement in the temp buffer and fix missing cell border. (Bug #64205)
---
 lisp/org/org-table.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
index 42f234790c5..6810fd8dc5a 100644
--- a/lisp/org/org-table.el
+++ b/lisp/org/org-table.el
@@ -6132,9 +6132,13 @@ supported."
   (with-temp-buffer
     (insert (orgtbl-to-orgtbl table params))
     (org-table-align)
-    (replace-regexp-in-string
-     "-|" "-+"
-     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
+    (goto-char (point-min))
+    (while (re-search-forward "-|" nil t)
+      (replace-match "-+"))
+    (goto-char (point-min))
+    (while (re-search-forward "|-" nil t)
+      (replace-match "+-"))
+    (buffer-string)))
 
 (defun orgtbl-to-unicode (table params)
   "Convert the `orgtbl-mode' TABLE into a table with unicode characters.
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
  2023-06-21 14:08 ` bug#64205: Fix missing cell border " Jakub Ječmínek
@ 2023-06-21 15:24   ` Robert Pluim
  2023-06-21 15:42     ` Jakub Ječmínek
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Pluim @ 2023-06-21 15:24 UTC (permalink / raw)
  To: Jakub Ječmínek; +Cc: 64205

>>>>> On Wed, 21 Jun 2023 16:08:13 +0200, Jakub Ječmínek <jecminek.k@gmail.com> said:
    Jakub> diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
    Jakub> index 42f234790c5..6810fd8dc5a 100644
    Jakub> --- a/lisp/org/org-table.el
    Jakub> +++ b/lisp/org/org-table.el
    Jakub> @@ -6132,9 +6132,13 @@ supported."
    Jakub>    (with-temp-buffer
    Jakub>      (insert (orgtbl-to-orgtbl table params))
    Jakub>      (org-table-align)
    Jakub> -    (replace-regexp-in-string
    Jakub> -     "-|" "-+"
    Jakub> -     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
    Jakub> +    (goto-char (point-min))
    Jakub> +    (while (re-search-forward "-|" nil t)
    Jakub> +      (replace-match "-+"))
    Jakub> +    (goto-char (point-min))
    Jakub> +    (while (re-search-forward "|-" nil t)
    Jakub> +      (replace-match "+-"))
    Jakub> +    (buffer-string)))

Youʼre replacing fixed strings, so you could use `search-forward'
instead of `re-search-forward'.

Robert
-- 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
  2023-06-21 15:24   ` Robert Pluim
@ 2023-06-21 15:42     ` Jakub Ječmínek
       [not found]       ` <CAFBiodLYe+98=dYnvYCqxW0yY1feR8t_OKEH9VLUYXvLRXpEAA@mail.gmail.com>
  0 siblings, 1 reply; 8+ messages in thread
From: Jakub Ječmínek @ 2023-06-21 15:42 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 64205


[-- Attachment #1.1: Type: text/plain, Size: 1315 bytes --]

You're right. There are more of these mistakes in the `org-table' file, for
example on line 1061, 1126 and 4666.

Jakub

st 21. 6. 2023 v 17:24 odesílatel Robert Pluim <rpluim@gmail.com> napsal:

> >>>>> On Wed, 21 Jun 2023 16:08:13 +0200, Jakub Ječmínek <
> jecminek.k@gmail.com> said:
>     Jakub> diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
>     Jakub> index 42f234790c5..6810fd8dc5a 100644
>     Jakub> --- a/lisp/org/org-table.el
>     Jakub> +++ b/lisp/org/org-table.el
>     Jakub> @@ -6132,9 +6132,13 @@ supported."
>     Jakub>    (with-temp-buffer
>     Jakub>      (insert (orgtbl-to-orgtbl table params))
>     Jakub>      (org-table-align)
>     Jakub> -    (replace-regexp-in-string
>     Jakub> -     "-|" "-+"
>     Jakub> -     (replace-regexp-in-string "|-" "+-" (buffer-substring 1
> (buffer-size))))))
>     Jakub> +    (goto-char (point-min))
>     Jakub> +    (while (re-search-forward "-|" nil t)
>     Jakub> +      (replace-match "-+"))
>     Jakub> +    (goto-char (point-min))
>     Jakub> +    (while (re-search-forward "|-" nil t)
>     Jakub> +      (replace-match "+-"))
>     Jakub> +    (buffer-string)))
>
> Youʼre replacing fixed strings, so you could use `search-forward'
> instead of `re-search-forward'.
>
> Robert
> --
>

[-- Attachment #1.2: Type: text/html, Size: 1996 bytes --]

[-- Attachment #2: 0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch --]
[-- Type: application/octet-stream, Size: 1239 bytes --]

From 61024954e75c4e71ed48c0566e02fb6e67b3e688 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jakub=20Je=C4=8Dm=C3=ADnek?= <jecminek.k@gmail.com>
Date: Wed, 21 Jun 2023 15:50:31 +0200
Subject: [PATCH] Fix orgtbl-to-table.el function to include last cell border

* lisp/org/org-table.el (orgtbl-to-table.el): Perform character
replacement in the temp buffer and fix missing cell border. (Bug #64205)
---
 lisp/org/org-table.el | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
index 42f234790c5..9a72eb5f314 100644
--- a/lisp/org/org-table.el
+++ b/lisp/org/org-table.el
@@ -6132,9 +6132,13 @@ supported."
   (with-temp-buffer
     (insert (orgtbl-to-orgtbl table params))
     (org-table-align)
-    (replace-regexp-in-string
-     "-|" "-+"
-     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
+    (goto-char (point-min))
+    (while (search-forward "-|" nil t)
+      (replace-match "-+"))
+    (goto-char (point-min))
+    (while (search-forward "|-" nil t)
+      (replace-match "+-"))
+    (buffer-string)))
 
 (defun orgtbl-to-unicode (table params)
   "Convert the `orgtbl-mode' TABLE into a table with unicode characters.
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
       [not found]       ` <CAFBiodLYe+98=dYnvYCqxW0yY1feR8t_OKEH9VLUYXvLRXpEAA@mail.gmail.com>
@ 2023-06-22 14:38         ` Robert Pluim
  2023-06-22 15:15           ` Eli Zaretskii
  0 siblings, 1 reply; 8+ messages in thread
From: Robert Pluim @ 2023-06-22 14:38 UTC (permalink / raw)
  To: Jakub Ječmínek; +Cc: 64205, Eli Zaretskii

>>>>> On Thu, 22 Jun 2023 16:33:02 +0200, Jakub Ječmínek <jecminek.k@gmail.com> said:

    Jakub> Tags: patch
    Jakub> I forgot to mention that I´ve fixed what you asked for, thank you for
    Jakub> pointing that out.

Thanks for that. Eli, is this small enough to go into master without
paperwork?

Thanks

Robert

    Jakub> See attached.

    Jakub> Jakub
    Jakub> From 61024954e75c4e71ed48c0566e02fb6e67b3e688 Mon Sep 17 00:00:00 2001
    Jakub> From: =?UTF-8?q?Jakub=20Je=C4=8Dm=C3=ADnek?= <jecminek.k@gmail.com>
    Jakub> Date: Wed, 21 Jun 2023 15:50:31 +0200
    Jakub> Subject: [PATCH] Fix orgtbl-to-table.el function to include last cell border

    Jakub> * lisp/org/org-table.el (orgtbl-to-table.el): Perform character
    Jakub> replacement in the temp buffer and fix missing cell border. (Bug #64205)
    Jakub> ---
    Jakub>  lisp/org/org-table.el | 10 +++++++---
    Jakub>  1 file changed, 7 insertions(+), 3 deletions(-)

    Jakub> diff --git a/lisp/org/org-table.el b/lisp/org/org-table.el
    Jakub> index 42f234790c5..9a72eb5f314 100644
    Jakub> --- a/lisp/org/org-table.el
    Jakub> +++ b/lisp/org/org-table.el
    Jakub> @@ -6132,9 +6132,13 @@ supported."
    Jakub>    (with-temp-buffer
    Jakub>      (insert (orgtbl-to-orgtbl table params))
    Jakub>      (org-table-align)
    Jakub> -    (replace-regexp-in-string
    Jakub> -     "-|" "-+"
    Jakub> -     (replace-regexp-in-string "|-" "+-" (buffer-substring 1 (buffer-size))))))
    Jakub> +    (goto-char (point-min))
    Jakub> +    (while (search-forward "-|" nil t)
    Jakub> +      (replace-match "-+"))
    Jakub> +    (goto-char (point-min))
    Jakub> +    (while (search-forward "|-" nil t)
    Jakub> +      (replace-match "+-"))
    Jakub> +    (buffer-string)))
 
    Jakub>  (defun orgtbl-to-unicode (table params)
    Jakub>    "Convert the `orgtbl-mode' TABLE into a table with unicode characters.
    Jakub> -- 
    Jakub> 2.39.1



Robert
-- 





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
  2023-06-22 14:38         ` Robert Pluim
@ 2023-06-22 15:15           ` Eli Zaretskii
  2023-06-22 15:43             ` Robert Pluim
  0 siblings, 1 reply; 8+ messages in thread
From: Eli Zaretskii @ 2023-06-22 15:15 UTC (permalink / raw)
  To: Robert Pluim; +Cc: 64205, jecminek.k

> From: Robert Pluim <rpluim@gmail.com>
> Cc: 64205@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Thu, 22 Jun 2023 16:38:48 +0200
> 
> >>>>> On Thu, 22 Jun 2023 16:33:02 +0200, Jakub Ječmínek <jecminek.k@gmail.com> said:
> 
>     Jakub> Tags: patch
>     Jakub> I forgot to mention that I´ve fixed what you asked for, thank you for
>     Jakub> pointing that out.
> 
> Thanks for that. Eli, is this small enough to go into master without
> paperwork?

Yes.  Just don't forget the Copyright-paperwork-exempt thingy.

Thanks.





^ permalink raw reply	[flat|nested] 8+ messages in thread

* bug#64205: Fix missing cell border when using orgtbl-to-table.el function
  2023-06-22 15:15           ` Eli Zaretskii
@ 2023-06-22 15:43             ` Robert Pluim
  0 siblings, 0 replies; 8+ messages in thread
From: Robert Pluim @ 2023-06-22 15:43 UTC (permalink / raw)
  To: jecminek.k; +Cc: 64205, Eli Zaretskii

tags 64205 fixed
close 64205 30.1
quit

>>>>> On Thu, 22 Jun 2023 18:15:17 +0300, Eli Zaretskii <eliz@gnu.org> said:

    >> From: Robert Pluim <rpluim@gmail.com>
    >> Cc: 64205@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
    >> Date: Thu, 22 Jun 2023 16:38:48 +0200
    >> 
    >> >>>>> On Thu, 22 Jun 2023 16:33:02 +0200, Jakub Ječmínek <jecminek.k@gmail.com> said:
    >> 
    Jakub> Tags: patch
    Jakub> I forgot to mention that I´ve fixed what you asked for, thank you for
    Jakub> pointing that out.
    >> 
    >> Thanks for that. Eli, is this small enough to go into master without
    >> paperwork?

    Eli> Yes.  Just don't forget the Copyright-paperwork-exempt thingy.

Closing.
Committed as 4c01b0deee1

For future changes, I suspect youʼll need to assign copyright to the
changes to the FSF, if youʼre willing to do so (the limit is approx 10
lines of code).

Thanks

Robert
-- 





^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-06-22 15:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-20 21:57 bug#64205: Fix missing border cell when using orgtbl-to-table.el function Jakub Ječmínek
2023-06-21 12:45 ` Robert Pluim
2023-06-21 14:08 ` bug#64205: Fix missing cell border " Jakub Ječmínek
2023-06-21 15:24   ` Robert Pluim
2023-06-21 15:42     ` Jakub Ječmínek
     [not found]       ` <CAFBiodLYe+98=dYnvYCqxW0yY1feR8t_OKEH9VLUYXvLRXpEAA@mail.gmail.com>
2023-06-22 14:38         ` Robert Pluim
2023-06-22 15:15           ` Eli Zaretskii
2023-06-22 15:43             ` Robert Pluim

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.