GNU bug report logs - #64205
Fix missing border cell when using orgtbl-to-table.el function

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 64205 in the body.
You can then email your comments to 64205 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Jakub Ječmínek <jecminek.k <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: Fix missing border cell when using orgtbl-to-table.el function
Date: Tue, 20 Jun 2023 23:57:35 +0200

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/html, inline)]

[0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch (application/octet-stream, attachment)]

Message #8 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Jakub Ječmínek <jecminek.k <at> gmail.com>
Cc: 64205 <at> debbugs.gnu.org
Subject: Re: bug#64205: Fix missing border cell when using
 orgtbl-to-table.el function
Date: Wed, 21 Jun 2023 14:45:59 +0200

>>>>> On Tue, 20 Jun 2023 23:57:35 +0200, Jakub Ječmínek <jecminek.k <at> 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
-- 

Message #11 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Jakub Ječmínek <jecminek.k <at> gmail.com>
To: 64205 <at> debbugs.gnu.org
Subject: Fix missing cell border when using orgtbl-to-table.el function
Date: Wed, 21 Jun 2023 16:08:13 +0200

[Message part 1 (text/plain, inline)]

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

[Message part 2 (text/html, inline)]

[0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch (application/octet-stream, attachment)]

Message #14 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Jakub Ječmínek <jecminek.k <at> gmail.com>
Cc: 64205 <at> debbugs.gnu.org
Subject: Re: bug#64205: Fix missing cell border when using
 orgtbl-to-table.el function
Date: Wed, 21 Jun 2023 17:24:47 +0200

>>>>> On Wed, 21 Jun 2023 16:08:13 +0200, Jakub Ječmínek <jecminek.k <at> 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
-- 

Message #17 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Jakub Ječmínek <jecminek.k <at> gmail.com>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 64205 <at> debbugs.gnu.org
Subject: Re: bug#64205: Fix missing cell border when using orgtbl-to-table.el
 function
Date: Wed, 21 Jun 2023 17:42:22 +0200

[Message part 1 (text/plain, inline)]

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 <at> gmail.com> napsal:

> >>>>> On Wed, 21 Jun 2023 16:08:13 +0200, Jakub Ječmínek <
> jecminek.k <at> 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
> --
>

[Message part 2 (text/html, inline)]

[0001-Fix-orgtbl-to-table.el-function-to-include-last-cell.patch (application/octet-stream, attachment)]

Message #20 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: Jakub Ječmínek <jecminek.k <at> gmail.com>
Cc: 64205 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#64205: Fix missing cell border when using
 orgtbl-to-table.el function
Date: Thu, 22 Jun 2023 16:38:48 +0200

>>>>> On Thu, 22 Jun 2023 16:33:02 +0200, Jakub Ječmínek <jecminek.k <at> 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 <at> 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
-- 

Message #23 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Robert Pluim <rpluim <at> gmail.com>
Cc: 64205 <at> debbugs.gnu.org, jecminek.k <at> gmail.com
Subject: Re: bug#64205: Fix missing cell border when using
 orgtbl-to-table.el function
Date: Thu, 22 Jun 2023 18:15:17 +0300

> From: Robert Pluim <rpluim <at> gmail.com>
> Cc: 64205 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> 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 <at> 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.

Message #26 received at 64205 <at> debbugs.gnu.org (full text, mbox):

From: Robert Pluim <rpluim <at> gmail.com>
To: jecminek.k <at> gmail.com
Cc: 64205 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#64205: Fix missing cell border when using
 orgtbl-to-table.el function
Date: Thu, 22 Jun 2023 17:43:58 +0200

tags 64205 fixed
close 64205 30.1
quit

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

    >> From: Robert Pluim <rpluim <at> gmail.com>
    >> Cc: 64205 <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> 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 <at> 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
-- 

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.