Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Simplify LaTeX mark-up for table column widths (ref #3379) #3381

Merged
merged 2 commits into from
Feb 4, 2017

Conversation

jfbu
Copy link
Contributor

@jfbu jfbu commented Feb 1, 2017

The \X token is used as column-specifier: this does not define or redefine \X as a LaTeX macro. Using a letter could have led to conflict with user custom column types or table packages. This column specifier takes two arguments which must be positive integers, as produced by LaTeX writer for :widths: option or for equal widths columns. As it always uses \linewidth the latter was not abstracted into a third argument.

There is no new functionality, but it shortens the produced .tex files.

The ``\X`` token is used as column-specifier: this does not define or
redefine ``\X`` as a LaTeX macro. Using a letter could have led to
conflict with user defined column types or table packages. This column
specifier takes two arguments which must be positive integers, as
produced by LaTeX writer for ``:widths:`` option or for equal widths
columns. As it always uses ``\linewidth`` the latter was not abstracted
into a third argument.
@jfbu jfbu requested a review from tk0miya February 1, 2017 19:09
@jfbu jfbu added this to the 1.6 milestone Feb 1, 2017
@tk0miya
Copy link
Member

tk0miya commented Feb 2, 2017

LGTM!

Also, organizes code for more clarity in conditionals.
@jfbu
Copy link
Contributor Author

jfbu commented Feb 2, 2017

@tk0miya thanks! may I ask you to check again : I have added commit with uses tabular, not tabulary in this case of use of :widths:. Indeed in this case the column widths are all know as a fraction of \linewidth hence use of tabulary is not needed.

Also, in passing I have a bit streamlined else and elif conditionals to make code simpler to read and I have removed a code comment which appears now un-necessary.

Memo: if there was a way for user to specify a target :width: additionally to :widths: this could now be handled easily, with a third argument to the \X columns type which will come in front of \linewidth. This mark-up:

.. table:: My Table
   :width: 80%
   :widths: 20,40,30

would mean that the table occupies 80% of available line width, in the 20:40:30 proportion. But as far as I know currently there is no such option in docutils. Sphinx could add a directive like tabularcolumns but I think it is not too good to add too much target specific directives. But there could be in the same spirit a tablewidth directive. Best would be if docutils allowed syntax like the above. (Together with the :align:, then we could try text flowing around tables like for figures; not easy in LaTeX but partially obtained with wrapfig).

memo2: I tested that our \X column specifier for tabular, tabulary, longtable does also work with tabu package. But tabu is not compatible with threeparttable.

@tk0miya
Copy link
Member

tk0miya commented Feb 4, 2017

LGTM again!

Your idea, the :width: option sounds good. How about propose it to docutils project? I will make patch.

@jfbu jfbu merged commit 6808e5a into sphinx-doc:master Feb 4, 2017
@jfbu
Copy link
Contributor Author

jfbu commented Feb 4, 2017

thanks @tk0miya for re-iterated review and +1 for proposing :width: to docutils. Now merged and CHANGES updated (a542ca2). I fixed an unrelated typo in CHANGES at 87ab6b0.

@jfbu jfbu deleted the 3379slightrewrite branch February 4, 2017 08:12
@jfbu jfbu mentioned this pull request Feb 4, 2017
jfbu added a commit to jfbu/sphinx that referenced this pull request Feb 4, 2017
tk0miya added a commit to tk0miya/sphinx that referenced this pull request Feb 4, 2017
@jfbu
Copy link
Contributor Author

jfbu commented Feb 9, 2017

about :width: option there is already a proposed patch at the doctutils issue tracker

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants