From 87db8ba5b8e43363e982c2e6de3fbf63f7694684 Mon Sep 17 00:00:00 2001 From: Juergen Spitzmueller Date: Tue, 27 Feb 2018 19:03:42 +0100 Subject: [PATCH] Fix RTL tabular output with bidi package (i.e., polyglossia) Fixes: #9686 (cherry picked from commit 21b347a2f8485ec3113dd7d76f8d78eb0cd426e2) --- src/insets/InsetTabular.cpp | 159 +++++++++++++++++++++++------------- src/insets/InsetTabular.h | 13 +-- status.23x | 2 + 3 files changed, 114 insertions(+), 60 deletions(-) diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp index 101bf333d9..c111365f28 100644 --- a/src/insets/InsetTabular.cpp +++ b/src/insets/InsetTabular.cpp @@ -2177,23 +2177,24 @@ bool Tabular::isPartOfMultiRow(row_type row, col_type column) const } -void Tabular::TeXTopHLine(otexstream & os, row_type row, string const & lang) const +void Tabular::TeXTopHLine(otexstream & os, row_type row, string const & lang, + list columns) const { // we only output complete row lines and the 1st row here, the rest // is done in Tabular::TeXBottomHLine(...) // get for each column the topline (if any) - vector topline; + map topline; col_type nset = 0; - for (col_type c = 0; c < ncols(); ++c) { - topline.push_back(topLine(cellIndex(row, c))); + for (auto const & c : columns) { + topline[c] = topLine(cellIndex(row, c)); // If cell is part of a multirow and not the first cell of the // multirow, no line must be drawn. if (row != 0) if (isMultiRow(cellIndex(row, c)) && cell_info[row][c].multirow != CELL_BEGIN_OF_MULTIROW) topline[c] = false; - if (topline[c]) + if (topline.find(c) != topline.end() && topline.find(c)->second) ++nset; } @@ -2209,8 +2210,8 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, string const & lang) co os << "\\hline "; } } else if (row == 0) { - for (col_type c = 0; c < ncols(); ++c) { - if (topline[c]) { + for (auto & c : columns) { + if (topline.find(c)->second) { col_type offset = 0; for (col_type j = 0 ; j < c; ++j) if (column_info[j].alignment == LYX_ALIGN_DECIMAL) @@ -2225,7 +2226,7 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, string const & lang) co os << (use_booktabs ? "\\cmidrule{" : "\\cline{") << c + 1 + offset << '-'; col_type cstart = c; - for ( ; c < ncols() && topline[c]; ++c) {} + for ( ; c < ncols() && topline.find(c)->second; ++c) {} for (col_type j = cstart ; j < c ; ++j) if (column_info[j].alignment == LYX_ALIGN_DECIMAL) @@ -2239,18 +2240,19 @@ void Tabular::TeXTopHLine(otexstream & os, row_type row, string const & lang) co } -void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang) const +void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang, + list columns) const { // we output bottomlines of row r and the toplines of row r+1 // if the latter do not span the whole tabular // get the bottomlines of row r, and toplines in next row bool lastrow = row == nrows() - 1; - vector bottomline, topline; + map bottomline, topline; bool nextrowset = true; - for (col_type c = 0; c < ncols(); ++c) { - bottomline.push_back(bottomLine(cellIndex(row, c))); - topline.push_back(!lastrow && topLine(cellIndex(row + 1, c))); + for (auto const & c : columns) { + bottomline[c] = bottomLine(cellIndex(row, c)); + topline[c] = !lastrow && topLine(cellIndex(row + 1, c)); // If cell is part of a multirow and not the last cell of the // multirow, no line must be drawn. if (!lastrow) @@ -2260,15 +2262,15 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang) bottomline[c] = false; topline[c] = false; } - nextrowset &= topline[c]; + nextrowset &= topline.find(c) != topline.end() && topline.find(c)->second; } // combine this row's bottom lines and next row's toplines if necessary col_type nset = 0; - for (col_type c = 0; c < ncols(); ++c) { + for (auto const & c : columns) { if (!nextrowset) - bottomline[c] = bottomline[c] || topline[c]; - if (bottomline[c]) + bottomline[c] = bottomline.find(c)->second || topline.find(c)->second; + if (bottomline.find(c)->second) ++nset; } @@ -2282,8 +2284,8 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang) else os << "\\hline "; } else { - for (col_type c = 0; c < ncols(); ++c) { - if (bottomline[c]) { + for (auto & c : columns) { + if (bottomline.find(c)->second) { col_type offset = 0; for (col_type j = 0 ; j < c; ++j) if (column_info[j].alignment == LYX_ALIGN_DECIMAL) @@ -2298,7 +2300,7 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang) os << (use_booktabs ? "\\cmidrule{" : "\\cline{") << c + 1 + offset << '-'; col_type cstart = c; - for ( ; c < ncols() && bottomline[c]; ++c) {} + for ( ; c < ncols() && bottomline.find(c)->second; ++c) {} for (col_type j = cstart ; j < c ; ++j) if (column_info[j].alignment == LYX_ALIGN_DECIMAL) @@ -2313,7 +2315,8 @@ void Tabular::TeXBottomHLine(otexstream & os, row_type row, string const & lang) void Tabular::TeXCellPreamble(otexstream & os, idx_type cell, - bool & ismulticol, bool & ismultirow) const + bool & ismulticol, bool & ismultirow, + bool const bidi) const { row_type const r = cellRow(cell); if (is_long_tabular && row_info[r].caption) @@ -2323,8 +2326,10 @@ void Tabular::TeXCellPreamble(otexstream & os, idx_type cell, LyXAlignment align = getAlignment(cell, !isMultiColumn(cell)); // figure out how to set the lines // we always set double lines to the right of the cell + // or left in bidi RTL, respectively. col_type const c = cellColumn(cell); col_type const nextcol = c + columnSpan(cell); + bool const decimal = column_info[c].alignment == LYX_ALIGN_DECIMAL; bool colright = columnRightLine(c); bool colleft = columnLeftLine(c); bool nextcolleft = nextcol < ncols() && columnLeftLine(nextcol); @@ -2333,28 +2338,35 @@ void Tabular::TeXCellPreamble(otexstream & os, idx_type cell, bool coldouble = colright && nextcolleft; bool celldouble = rightLine(cell) && nextcellleft; - ismulticol = isMultiColumn(cell) - || (c == 0 && colleft != leftLine(cell)) - || ((colright || nextcolleft) && !rightLine(cell) && !nextcellleft) - || (!colright && !nextcolleft && (rightLine(cell) || nextcellleft)) - || (coldouble != celldouble); + ismulticol = (isMultiColumn(cell) + || (c == 0 && colleft != leftLine(cell)) + || ((colright || nextcolleft) && !rightLine(cell) && !nextcellleft) + || (!colright && !nextcolleft && (rightLine(cell) || nextcellleft)) + || (coldouble != celldouble)) + && !decimal; // we center in multicol when no decimal point - if (column_info[c].alignment == LYX_ALIGN_DECIMAL) { + if (decimal) { docstring const align_d = column_info[c].decimal_point; DocIterator const dit = separatorPos(cellInset(cell), align_d); - ismulticol |= !dit; + bool const nosep = !dit; + ismulticol |= nosep; + celldouble &= nosep; } // up counter by 1 for each decimally aligned col since they use 2 latex cols int latexcolspan = columnSpan(cell); - for(col_type col = c; col < c + columnSpan(cell); ++col) + for (col_type col = c; col < c + columnSpan(cell); ++col) if (column_info[col].alignment == LYX_ALIGN_DECIMAL) ++latexcolspan; if (ismulticol) { os << "\\multicolumn{" << latexcolspan << "}{"; - if (c ==0 && leftLine(cell)) + if (((bidi && c == getLastCellInRow(cellRow(0)) && rightLine(cell)) + || (!bidi && c == 0 && leftLine(cell)))) + os << '|'; + if (bidi && celldouble) + // add extra vertical line if we want a double one os << '|'; if (!cellInfo(cell).align_special.empty()) { os << cellInfo(cell).align_special; @@ -2401,9 +2413,9 @@ void Tabular::TeXCellPreamble(otexstream & os, idx_type cell, } } // end if else !getPWidth } // end if else !cellinfo_of_cell - if (rightLine(cell) || nextcellleft) + if ((bidi && leftLine(cell)) || (!bidi && rightLine(cell)) || nextcellleft) os << '|'; - if (celldouble) + if (!bidi && celldouble) // add extra vertical line if we want a double one os << '|'; os << "}{"; @@ -2499,7 +2511,8 @@ void Tabular::TeXCellPostamble(otexstream & os, idx_type cell, void Tabular::TeXLongtableHeaderFooter(otexstream & os, - OutputParams const & runparams) const + OutputParams const & runparams, + list columns) const { if (!is_long_tabular) return; @@ -2511,7 +2524,7 @@ void Tabular::TeXLongtableHeaderFooter(otexstream & os, if (row_info[r].caption && !row_info[r].endfirsthead && !row_info[r].endhead && !row_info[r].endfoot && !row_info[r].endlastfoot) - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); } } // output first header info @@ -2520,7 +2533,7 @@ void Tabular::TeXLongtableHeaderFooter(otexstream & os, os << "\\hline\n"; for (row_type r = 0; r < nrows(); ++r) { if (row_info[r].endfirsthead) - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); } if (endfirsthead.bottomDL) os << "\\hline\n"; @@ -2534,7 +2547,7 @@ void Tabular::TeXLongtableHeaderFooter(otexstream & os, os << "\\hline\n"; for (row_type r = 0; r < nrows(); ++r) { if (row_info[r].endhead) - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); } if (endhead.bottomDL) os << "\\hline\n"; @@ -2546,7 +2559,7 @@ void Tabular::TeXLongtableHeaderFooter(otexstream & os, os << "\\hline\n"; for (row_type r = 0; r < nrows(); ++r) { if (row_info[r].endfoot) - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); } if (endfoot.bottomDL) os << "\\hline\n"; @@ -2560,7 +2573,7 @@ void Tabular::TeXLongtableHeaderFooter(otexstream & os, os << "\\hline\n"; for (row_type r = 0; r < nrows(); ++r) { if (row_info[r].endlastfoot) - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); } if (endlastfoot.bottomDL) os << "\\hline\n"; @@ -2580,7 +2593,8 @@ bool Tabular::isValidRow(row_type row) const void Tabular::TeXRow(otexstream & os, row_type row, - OutputParams const & runparams) const + OutputParams const & runparams, + list columns) const { idx_type cell = cellIndex(row, 0); InsetTableCell const * inset = cellInset(cell); @@ -2588,7 +2602,7 @@ void Tabular::TeXRow(otexstream & os, row_type row, string const lang = par.getParLanguage(buffer().params())->lang(); //output the top line - TeXTopHLine(os, row, lang); + TeXTopHLine(os, row, lang, columns); if (row_info[row].top_space_default) { if (use_booktabs) @@ -2608,7 +2622,15 @@ void Tabular::TeXRow(otexstream & os, row_type row, } bool ismulticol = false; bool ismultirow = false; - for (col_type c = 0; c < ncols(); ++c) { + + // The bidi package (loaded by polyglossia) reverses RTL table columns + bool const bidi_rtl = + runparams.local_font->isRightToLeft() + && runparams.use_polyglossia; + idx_type lastcell = + bidi_rtl ? getFirstCellInRow(row) : getLastCellInRow(row); + + for (auto const & c : columns) { if (isPartOfMultiColumn(row, c)) continue; @@ -2616,12 +2638,12 @@ void Tabular::TeXRow(otexstream & os, row_type row, if (isPartOfMultiRow(row, c) && column_info[c].alignment != LYX_ALIGN_DECIMAL) { - if (cell != getLastCellInRow(row)) + if (cell != lastcell) os << " & "; continue; } - TeXCellPreamble(os, cell, ismulticol, ismultirow); + TeXCellPreamble(os, cell, ismulticol, ismultirow, bidi_rtl); InsetTableCell const * inset = cellInset(cell); Paragraph const & par = inset->paragraphs().front(); @@ -2663,14 +2685,24 @@ void Tabular::TeXRow(otexstream & os, row_type row, head.setMacrocontextPositionRecursive(dit); bool hassep = false; InsetTableCell tail = splitCell(head, column_info[c].decimal_point, hassep); - head.latex(os, newrp); if (hassep) { - os << '&'; tail.setBuffer(head.buffer()); dit.pop_back(); dit.push_back(CursorSlice(tail)); tail.setMacrocontextPositionRecursive(dit); - tail.latex(os, newrp); + } + if (bidi_rtl) { + if (hassep) { + tail.latex(os, newrp); + os << '&'; + } + head.latex(os, newrp); + } else { + head.latex(os, newrp); + if (hassep) { + os << '&'; + tail.latex(os, newrp); + } } } else if (ltCaption(row)) { // Inside longtable caption rows, we must only output the caption inset @@ -2694,7 +2726,7 @@ void Tabular::TeXRow(otexstream & os, row_type row, os << '}'; TeXCellPostamble(os, cell, ismulticol, ismultirow); - if (cell != getLastCellInRow(row)) { // not last cell in row + if (cell != lastcell) { // not last cell in row if (runparams.nice) os << " & "; else @@ -2717,7 +2749,7 @@ void Tabular::TeXRow(otexstream & os, row_type row, os << '\n'; //output the bottom line - TeXBottomHLine(os, row, lang); + TeXBottomHLine(os, row, lang, columns); if (row_info[row].interline_space_default) { if (use_booktabs) @@ -2792,8 +2824,21 @@ void Tabular::latex(otexstream & os, OutputParams const & runparams) const if (is_tabular_star) os << "@{\\extracolsep{\\fill}}"; - for (col_type c = 0; c < ncols(); ++c) { - if (columnLeftLine(c)) + // The bidi package (loaded by polyglossia) swaps the column + // order for RTL (#9686). Thus we use this list. + bool const bidi_rtl = + runparams.local_font->isRightToLeft() + && runparams.use_polyglossia; + list columns; + for (col_type cl = 0; cl < ncols(); ++cl) { + if (bidi_rtl) + columns.push_front(cl); + else + columns.push_back(cl); + } + + for (auto const & c : columns) { + if ((bidi_rtl && columnRightLine(c)) || (!bidi_rtl && columnLeftLine(c))) os << '|'; if (!column_info[c].align_special.empty()) { os << column_info[c].align_special; @@ -2815,11 +2860,15 @@ void Tabular::latex(otexstream & os, OutputParams const & runparams) const case LYX_ALIGN_LAYOUT: case LYX_ALIGN_SPECIAL: break; - case LYX_ALIGN_DECIMAL: - os << ">{\\raggedleft}"; + case LYX_ALIGN_DECIMAL: { + if (bidi_rtl) + os << ">{\\raggedright}"; + else + os << ">{\\raggedleft}"; decimal = true; break; } + } char valign = 'p'; switch (column_info[c].valignment) { @@ -2874,12 +2923,12 @@ void Tabular::latex(otexstream & os, OutputParams const & runparams) const } } // end if else !column_info[i].p_width } // end if else !column_info[i].align_special - if (columnRightLine(c)) + if ((bidi_rtl && columnLeftLine(c)) || (!bidi_rtl && columnRightLine(c))) os << '|'; } os << "}\n"; - TeXLongtableHeaderFooter(os, runparams); + TeXLongtableHeaderFooter(os, runparams, columns); //+--------------------------------------------------------------------- //+ the single row and columns (cells) + @@ -2887,7 +2936,7 @@ void Tabular::latex(otexstream & os, OutputParams const & runparams) const for (row_type r = 0; r < nrows(); ++r) { if (isValidRow(r)) { - TeXRow(os, r, runparams); + TeXRow(os, r, runparams, columns); if (is_long_tabular && row_info[r].newpage) os << "\\newpage\n"; } diff --git a/src/insets/InsetTabular.h b/src/insets/InsetTabular.h index 605815aa03..be11b0413e 100644 --- a/src/insets/InsetTabular.h +++ b/src/insets/InsetTabular.h @@ -800,20 +800,23 @@ public: /// // helper function for Latex /// - void TeXTopHLine(otexstream &, row_type row, std::string const & lang) const; + void TeXTopHLine(otexstream &, row_type row, std::string const & lang, + std::list) const; /// - void TeXBottomHLine(otexstream &, row_type row, std::string const & lang) const; + void TeXBottomHLine(otexstream &, row_type row, std::string const & lang, + std::list) const; /// - void TeXCellPreamble(otexstream &, idx_type cell, bool & ismulticol, bool & ismultirow) const; + void TeXCellPreamble(otexstream &, idx_type cell, bool & ismulticol, bool & ismultirow, + bool const bidi) const; /// void TeXCellPostamble(otexstream &, idx_type cell, bool ismulticol, bool ismultirow) const; /// - void TeXLongtableHeaderFooter(otexstream &, OutputParams const &) const; + void TeXLongtableHeaderFooter(otexstream &, OutputParams const &, std::list) const; /// bool isValidRow(row_type const row) const; /// void TeXRow(otexstream &, row_type const row, - OutputParams const &) const; + OutputParams const &, std::list) const; /// // helper functions for plain text /// diff --git a/status.23x b/status.23x index 96fe476364..fea862abe2 100644 --- a/status.23x +++ b/status.23x @@ -91,6 +91,8 @@ What's new - Fix output of table cells with multiple languages (bug 11128). +- Fix RTL table output with bidi package (non-TeX fonts) (bug 9686). + * USER INTERFACE