* [PATCH] lisp/org-table.el: Use booktabs on org-table-export
@ 2022-05-09 22:29 Pride Allman
2022-05-11 12:19 ` Ihor Radchenko
0 siblings, 1 reply; 4+ messages in thread
From: Pride Allman @ 2022-05-09 22:29 UTC (permalink / raw)
To: emacs-orgmode
[-- Attachment #1.1: Type: text/plain, Size: 1711 bytes --]
Hello,
This is my first patch so hopefully I followed the instructions correctly,
While using ~org-table-export~ on a table directly even with
~org-latex-tables-booktabs~ set to ~t~, it exports a normal table instead
of booktabs table. But on the same situation, if you export the whole
buffer the table will be exported according to booktabs.
So my theory was there is a discrepancy between the method used by
~org-latex-export-to-latex~ and the ~org-table-export~. And on browsing the
codes for those two I found this.
The code for ~org-latex-table-row~ used by latex export backend has this
line:
#+begin_src emacs-lisp :tangle yes
(booktabsp (if (plist-member attr :booktabs) (plist-get attr :booktabs)
(plist-get info :latex-tables-booktabs)))
#+end_src
Shows that it also takes into account the ~info~ plist.
I couldn't find out what that info plist has, but I can assume it's the
information on the overall buffer or that table's context, and it has the
effect of using the booktabs from the config.
While the code for ~orgtbl-to-latex~ shows it only takes into account the
~booktabs~ parameter on latex table.
#+begin_src emacs-lisp :tangle yes
:latex-tables-booktabs (plist-get params :booktabs)
#+end_src
So I tried changing the line in ~orgtbl-to-latex~ to
#+begin_src emacs-lisp :tangle yes
:latex-tables-booktabs (if (plist-member params :booktabs) (plist-get
params :booktabs)
org-latex-tables-booktabs)
#+end_src
So if there is ~booktabs~ parameter then it'll overwrite the settings but
if not it'll use the booktabs config.
There is probably a better way to use it, but this works for now so I tried
this.
Please find the attached patch with the fix.
Regards,
Gaurav
[-- Attachment #1.2: Type: text/html, Size: 2003 bytes --]
[-- Attachment #2: 0001-lisp-org-table.el-Use-booktabs-on-org-table-export.patch --]
[-- Type: text/x-patch, Size: 1528 bytes --]
From 264fb8b7f2c5782c92c5beffe54ac18c97b4b685 Mon Sep 17 00:00:00 2001
From: Gaurav <allmanpride@gmail.com>
Date: Mon, 9 May 2022 18:05:52 -0400
Subject: [PATCH] lisp/org-table.el: Use booktabs on org-table-export
* lisp/org-table.el (orgtbl-to-latex): Read booktabs flag from
`org-latex-tables-booktabs' if `:booktabs' is not present in
table `params'.
Problem was while using ~org-table-export~ on a table directly even
with ~org-latex-tables-booktabs~ set to ~t~, it exports a normal table
instead of booktabs table. But on the same point, if you export the
whole buffer the table will be exported according to booktabs. I
looked at the discrepancy between the method used by
~org-latex-export-to-latex~ and the ~org-table-export~ and did this to
fix it.
TINYCHANGE
---
lisp/org-table.el | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lisp/org-table.el b/lisp/org-table.el
index b160dc97c..46498f119 100644
--- a/lisp/org-table.el
+++ b/lisp/org-table.el
@@ -6000,7 +6000,9 @@ supported. It is also possible to use the following ones:
(list :backend 'latex
:latex-default-table-mode 'table
:latex-tables-centered nil
- :latex-tables-booktabs (plist-get params :booktabs)
+ :latex-tables-booktabs (if (plist-member params :booktabs)
+ (plist-get params :booktabs)
+ org-latex-tables-booktabs)
:latex-table-scientific-notation nil
:latex-default-table-environment
(or (plist-get params :environment) "tabular"))
--
2.36.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] lisp/org-table.el: Use booktabs on org-table-export
2022-05-09 22:29 [PATCH] lisp/org-table.el: Use booktabs on org-table-export Pride Allman
@ 2022-05-11 12:19 ` Ihor Radchenko
2022-05-11 17:01 ` Pride Allman
0 siblings, 1 reply; 4+ messages in thread
From: Ihor Radchenko @ 2022-05-11 12:19 UTC (permalink / raw)
To: Pride Allman; +Cc: emacs-orgmode
Pride Allman <allmanpride@gmail.com> writes:
> This is my first patch so hopefully I followed the instructions correctly,
Thanks for the patch! Patches are always welcome, though I do not fully
agree with the approach you used in this particular one.
> While using ~org-table-export~ on a table directly even with
> ~org-latex-tables-booktabs~ set to ~t~, it exports a normal table instead
> of booktabs table. But on the same situation, if you export the whole
> buffer the table will be exported according to booktabs.
I also find this discrepancy awkward.
Normally, Org export options are chosen according to
org-export-options-alist (see the docstring) and the equivalent
variables in specific export backend (see org-export-define-backend
'latex in ox-latex.el).
However, when you look into orgtbl-to-latex, orgtbl-to-html,
orgtbl-to-texinfo, and orgtbl-to-unicode, they all overwrite certain
export settings by force. Sometimes, even not leaving the user an option
to choose. All those (list :option value ...) are force-overwriting the
_global_ export defaults (see orgtbl-to-latex calling orgtbl-to-generic
that calls org-export-get-environment with third argument) , including
:latex-tables-booktabs. This is by design.
While I do not like the current design, your patch will only create even
more inconsistencies; The same problem will remain for other
force-overwritten options.
Best,
Ihor
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lisp/org-table.el: Use booktabs on org-table-export
2022-05-11 12:19 ` Ihor Radchenko
@ 2022-05-11 17:01 ` Pride Allman
2022-05-12 9:29 ` Ihor Radchenko
0 siblings, 1 reply; 4+ messages in thread
From: Pride Allman @ 2022-05-11 17:01 UTC (permalink / raw)
To: Ihor Radchenko; +Cc: emacs-orgmode
[-- Attachment #1: Type: text/plain, Size: 2266 bytes --]
So basically, the export function is doing the overwrite for user options
but to reduce the number of inconsistency we can't use this patch for only
one such option.
Now you explained it I can't think of a way to solve it without rewriting
the whole thing. Or maybe I need to read the source code more deeply.
Would it be too much to hope for it to be integrated on the basis that
things like this can be added for other options in other export backend
too? Or somehow introduce the global settings there.
And considering the answer is no for the previous question; is there a
workaround for this? Something people use? Because having to manually edit
the table everytime I export is a bit too much for me.
Thank you.
On Wed, 11 May 2022, 08:19 Ihor Radchenko, <yantar92@gmail.com> wrote:
> Pride Allman <allmanpride@gmail.com> writes:
>
> > This is my first patch so hopefully I followed the instructions
> correctly,
>
> Thanks for the patch! Patches are always welcome, though I do not fully
> agree with the approach you used in this particular one.
>
> > While using ~org-table-export~ on a table directly even with
> > ~org-latex-tables-booktabs~ set to ~t~, it exports a normal table instead
> > of booktabs table. But on the same situation, if you export the whole
> > buffer the table will be exported according to booktabs.
>
> I also find this discrepancy awkward.
>
> Normally, Org export options are chosen according to
> org-export-options-alist (see the docstring) and the equivalent
> variables in specific export backend (see org-export-define-backend
> 'latex in ox-latex.el).
>
> However, when you look into orgtbl-to-latex, orgtbl-to-html,
> orgtbl-to-texinfo, and orgtbl-to-unicode, they all overwrite certain
> export settings by force. Sometimes, even not leaving the user an option
> to choose. All those (list :option value ...) are force-overwriting the
> _global_ export defaults (see orgtbl-to-latex calling orgtbl-to-generic
> that calls org-export-get-environment with third argument) , including
> :latex-tables-booktabs. This is by design.
>
> While I do not like the current design, your patch will only create even
> more inconsistencies; The same problem will remain for other
> force-overwritten options.
>
> Best,
> Ihor
>
[-- Attachment #2: Type: text/html, Size: 2969 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lisp/org-table.el: Use booktabs on org-table-export
2022-05-11 17:01 ` Pride Allman
@ 2022-05-12 9:29 ` Ihor Radchenko
0 siblings, 0 replies; 4+ messages in thread
From: Ihor Radchenko @ 2022-05-12 9:29 UTC (permalink / raw)
To: Pride Allman; +Cc: emacs-orgmode
Pride Allman <allmanpride@gmail.com> writes:
> So basically, the export function is doing the overwrite for user options
> but to reduce the number of inconsistency we can't use this patch for only
> one such option.
>
> Now you explained it I can't think of a way to solve it without rewriting
> the whole thing. Or maybe I need to read the source code more deeply.
> Would it be too much to hope for it to be integrated on the basis that
> things like this can be added for other options in other export backend
> too? Or somehow introduce the global settings there.
As an option, the override arguments can be factored out into
defcustoms. Then, you would have an option to deal with the problem you
encountered.
Another possibility comes from the fact that the overrides are mostly
copying the defaults or trying to transfer alternative option names into
export options:
(list :backend 'latex
:latex-default-table-mode 'table ;; <- this is default
:latex-tables-centered nil <- this is _not_ default. The current default is t
:latex-tables-booktabs (plist-get params :booktabs) <- default is nil
:latex-table-scientific-notation nil <- this is also default
:latex-default-table-environment <- default is "tabular" but :environment is transferred
(or (plist-get params :environment) "tabular"))
Then, we can:
1. Remove options directly overriding the defaults
2. Do not assign :latex-tables-booktabs and
:latex-default-table-environment unless :booktabs and :environment
options are actually provided. Currently, when the latter options are
not set by user (via #+attr_latex), they are treated as if the user
force-set them to nil (because plist-get does not distinguish
(:property nil) and complete absence of :property).
I like the second possibility better because it will make table export
consistent with the rest of export customisation. The downside is that
we can break the existing workflows relying on current behaviour (which
is not too much of a big deal though - we can always document this
change in ORG-NEWS).
The first possibility, on the other hand, will not break the current
behaviour. However, it will only help people who manage to find this new
customisation. We already have too many customisations.
> And considering the answer is no for the previous question; is there a
> workaround for this? Something people use? Because having to manually edit
> the table everytime I export is a bit too much for me.
Yes, there should be a workaround. org-table functions only override
global export settings, not file-local and element-local. That is you
should be able to set #+bind: org-latex-tables-booktabs
org-latex-tables-booktabs. Or #+attr_latex: :booktabs t
Of course, it is not very intuitive and relies on internal knowledge of
the code.
Best,
Ihor
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2022-05-12 9:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-09 22:29 [PATCH] lisp/org-table.el: Use booktabs on org-table-export Pride Allman
2022-05-11 12:19 ` Ihor Radchenko
2022-05-11 17:01 ` Pride Allman
2022-05-12 9:29 ` Ihor Radchenko
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs/org-mode.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).