On 17.08.2017 12:47, racoon wrote:
On 17.08.2017 12:11, racoon wrote:
On 05.06.2017 05:32, Scott Kostyshak wrote:
On Thu, May 25, 2017 at 09:45:52AM -0400, Scott Kostyshak wrote:
On Wed, May 24, 2017 at 08:07:52AM +0200, racoon wrote:

Thanks. Yes, it is still not patched in latest master so I guess it is
pending.

Feel free to take a look.

OK I'll put this on my list of things to look at.

Patch is in at b1ddae05. Your patch fixed #9306 and #10538.

Just saw that the patch made it into beta1. Thanks.

Now I am wondering whether the same logic as for rows should be applied analogously for columns. At the moment they are different. I could take a look at it.

Another thing I noticed is that the "set all lines" button does what it says. It turns on all lines which leads to double lines. However, I think this might not be what most users want (and it is also not reflected by the button icon). I suggest that instead it turns on all left and top lines plus the right and bottom lines in the last row and column, respectively. This is the default on inserting tables and plays along nicely with the add column line algorithm.

I could look at that too, if desired.

Attached is a patch for lines when adding a row. It is just analogous to the column addition logic replacing the respective lines:

1. top <-> left,
2. right <-> bottom

Looking at the code, also revealed that the logic for adding rows/columns for multirows/-columns isn't intuitive.

Steps to reproduce:
1. Open attached lyx-file (it created with 2.3.0dev)
2. Put the cursor in the multicolumn.
3. Add a column.

I'll have a look at that too.

Attached is a patch that in addition to the previous one also fixed double lines created by the set all and border lines function. The new function never creates double lines *given* that one uses only those two functions. I think it is an improvement.

However, I think ultimately double lines and single lines should be separate functions such that one gets double lines only when explicitly required. (See also writer apps like Libre, etc.)

>From 9e082ceeb1fa18279d0f99c5f80c76688d00f782 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Ram=C3=B6ller?= <d....@web.de>
Date: Thu, 17 Aug 2017 13:50:51 +0930
Subject: [PATCH 2/2] Fix for double lines created by "set all lines" and "set
 border lines".

---
 src/insets/InsetTabular.cpp | 38 ++++++++++++++++++++++++++++++--------
 1 file changed, 30 insertions(+), 8 deletions(-)

diff --git a/src/insets/InsetTabular.cpp b/src/insets/InsetTabular.cpp
index 0ec488c571..029656f598 100644
--- a/src/insets/InsetTabular.cpp
+++ b/src/insets/InsetTabular.cpp
@@ -5437,7 +5437,6 @@ void InsetTabular::tabularFeatures(Cursor & cur,
        col_type sel_col_end;
        row_type sel_row_start;
        row_type sel_row_end;
-       bool setLines = false;
        LyXAlignment setAlign = LYX_ALIGN_LEFT;
        Tabular::VAlignment setVAlign = Tabular::LYX_VALIGN_TOP;
 
@@ -5781,26 +5780,49 @@ void InsetTabular::tabularFeatures(Cursor & cur,
        }
 
        case Tabular::SET_ALL_LINES:
-               setLines = true;
+               for (row_type r = sel_row_start; r <= sel_row_end; ++r)
+                       for (col_type c = sel_col_start; c <= sel_col_end; ++c) 
{
+                               idx_type const cell = tabular.cellIndex(r, c);
+                               tabular.setTopLine(cell, true);
+                               if (r == sel_row_end) {
+                                       if (r == tabular.nrows() - 1)
+                                               tabular.setBottomLine(cell, 
true);
+                                       else
+                                               
tabular.setTopLine(tabular.cellIndex(r + 1, c), true);
+                               }
+                               tabular.setLeftLine(cell, true);
+                               if (c == sel_col_end)
+                                       if (c == tabular.ncols() - 1)
+                                               tabular.setRightLine(cell, 
true);
+                                       else
+                                               
tabular.setLeftLine(tabular.cellIndex(r, c + 1), true);
+                       }
+               break;
        case Tabular::UNSET_ALL_LINES:
                for (row_type r = sel_row_start; r <= sel_row_end; ++r)
                        for (col_type c = sel_col_start; c <= sel_col_end; ++c) 
{
                                idx_type const cell = tabular.cellIndex(r, c);
-                               tabular.setTopLine(cell, setLines);
-                               tabular.setBottomLine(cell, setLines);
-                               tabular.setRightLine(cell, setLines);
-                               tabular.setLeftLine(cell, setLines);
+                               tabular.setTopLine(cell, false);
+                               tabular.setBottomLine(cell, false);
+                               tabular.setRightLine(cell, false);
+                               tabular.setLeftLine(cell, false);
                        }
                break;
 
        case Tabular::SET_BORDER_LINES:
                for (row_type r = sel_row_start; r <= sel_row_end; ++r) {
                        tabular.setLeftLine(tabular.cellIndex(r, 
sel_col_start), true);
-                       tabular.setRightLine(tabular.cellIndex(r, sel_col_end), 
true);
+                       if (sel_col_end == tabular.ncols() - 1)
+                               tabular.setRightLine(tabular.cellIndex(r, 
sel_col_end), true);
+                       else
+                               tabular.setLeftLine(tabular.cellIndex(r, 
sel_col_end + 1), true);
                }
                for (col_type c = sel_col_start; c <= sel_col_end; ++c) {
                        tabular.setTopLine(tabular.cellIndex(sel_row_start, c), 
true);
-                       tabular.setBottomLine(tabular.cellIndex(sel_row_end, 
c), true);
+                       if (sel_row_end == tabular.nrows() - 1)
+                               
tabular.setBottomLine(tabular.cellIndex(sel_row_end, c), true);
+                       else
+                               
tabular.setTopLine(tabular.cellIndex(sel_row_end + 1, c), true);
                }
                break;
 
-- 
2.11.1.windows.1

Reply via email to