Patch 1

[willflet]

Fix interactions between colspan and rowspan

Fixed two problems that were not addressed in previous patch:

1) Row-spanning cells '^^' would reference the wrong spancell if any previous cells in that row had width>1. This includes cells where its referenced spancell had width>1. Issue resolved by incrementing col_index using the correct width.

2) Row-spanning cells '^^' that use multiple pipe delimiters (e.g. for consistency with the referenced column-spanning cell) would cause excess cells in the row to be removed. Issue resolved by skipping the pipe-counting logic when cell is also row spanning '^^' , then removing the correct number of excess columns when handling the rowspan.

[jeffreytse]

Hi @willflet Thank you for the positive effect to this project.

Current design is to keep consistency with the HTML table operating behaviour, we use | by | to do a colspan (i.e. to merge right side cell ), and we use ^^ to do a rowspan (i.e. merge to above cell and the above cell rowspan increase 1), it means as below:

1) If you merge right cell, the column number of current row will reduce 1. 2) If you merge to above cell, the row number of current column will reduce 1. 3) If two actions in the same cell, it will merge right side and then merge to above cell.

i.e. If you want to do a rowspan with the cell of previous row, you need to keep the same with above cell, the colspan process can't be skipped even though current cell need to merge to the cell of previous row.

For example:

1) All is same column number.

|    1   |    2   |    3    |   4    |    5    |    6    |   7    |
|    spancell1   ||    spancell 2   ||  cell   |    spancell3    ||
| ^^  spancell1  ||    spancell 2   ||  cell   |    spancell3    ||

2) One more column in the third row. (i.e. No merging to right side for the col 1)

|    1   |    2   |    3    |   4    |    5    |    6    |   7    |
|    spancell1   ||    spancell 2   ||  cell   |    spancell3    ||
| ^^  spancell1   |    spancell 2   ||  cell   |    spancell3    ||

3) One column lacked in the third row. (i.e. one more merging to right side for the col 1)

|    1   |    2   |    3    |   4    |    5    |    6    |   7    |
|    spancell1   ||    spancell 2   ||  cell   |    spancell3    ||
| ^^  spancell1 |||    spancell 2   ||  cell   |    spancell3    ||

[willflet]

Thanks for reviewing, your cleanup of the previous commit is a definite improvement :)

As for this issue, it still needs my patch even with the correct number of pipes. It's a side-effect that the number of pipes in ^^ cells is now ignored. Using the current state of the master branch,

|:-----:|:-----:|:-----:|:-----:| | (0,0) | (0,1) | (0,2) | (0,3) | | (1,0) || ^^ | (1,3) |

gives

(not sure why the borders render like that, either)

With my patch it gives the correct behaviour:

Let me know if you need more explanation, hopefully you are on the same page after digesting this + the commit message.

I appreciate your thoughts on design consistency, it's fair enough to use the pipe count instead of inferring the width from the above cell if you want. That depends if you think the behaviour you described is useful or an inconvenience. Personally I think the user will never want the result from (2) or (3) so I'd keep ignoring it.

On Fri, 15 May 2020 at 10:25, jeffreytse notifications@github.com wrote:

Hi @willflet https://github.com/willflet Thank you for the positive effect to this project.

Current design is to keep consistency with the HTML table operating behaviour, we use | by | to do a colspan (i.e. to merge right side cell ), and we use ^^ to do a rowspan (i.e. merge to above cell and above cell rowspan increase 1), it means as below:

1. If you merge right cell, the column number of current row will reduce 1.
2. If you merge to above cell, the row number of current column will reduce 1.

i.e. If you want to do a rowspan with the cell of previous row, you need to keep the same with above cell, the colspan process can't be skipped even though current cell need to merge to the cell of previous row.

For example:

1. All is same column number.

| 1 | 2 | 3 | 4 | 5 | 6 | 7 | | spancell1 || spancell 2 || cell | spancell3 || | ^^ spancell1 || spancell 2 || cell | spancell3 ||

1. One more column in the third row. (i.e. No merging to right side for the col 1)

| 1 | 2 | 3 | 4 | 5 | 6 | 7 | | spancell1 || spancell 2 || cell | spancell3 || | ^^ spancell1 | spancell 2 || cell | spancell3 ||

1. One column lacking in the third row. (i.e. one more merging to right side for the col 1)

| 1 | 2 | 3 | 4 | 5 | 6 | 7 | | spancell1 || spancell 2 || cell | spancell3 || | ^^ spancell1 ||| spancell 2 || cell | spancell3 ||

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/jeffreytse/jekyll-spaceship/pull/5#issuecomment-629131884, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGT3G6DFZMUSG5IALVO53ULRRUDANANCNFSM4NA6E3RA .

[jeffreytse]

Hi @willflet Thanks for the concrete description about the issue on interactions between colspan and rowspan. And this issue already was addressed and fixed.

[jeffreytse]

Hi @willflet This pr will be closed if you don't have any other questions, and welcome to commit new pr if you have any issues or new features.