DiffRender.render doesn't seem to set class on added cells in modified rows

[Floppy]

As found when working on gitlabhq/gitlabhq#4810:

Compare these two screenshots. The first is using handsontable to render, the second using DiffRender.render() to get html.

As you can see, in the first one, on the last two rows, fields have been modified, but the central column has also been added.

In the second case, using DiffRender.render() directly, no class is applied to the added column on the last two rows. The HTML looks like:

<tr>
  <td>+</td>
  <td>Neptune</td>
  <td class="add">4553946490</td>
  <td>11.28</td>
</tr>
<tr>
  <td class="modify">-&gt;</td>
  <td>Triton</td>
  <td></td>
  <td class="modify"> 0.779-&gt;0.779</td>
</tr>
<tr>
  <td class="modify">-&gt;</td>
  <td>Pluto</td>
  <td>7311000000</td>
  <td class="modify"> 0.61-&gt;0.61</td>
</tr>

I would expect the third td in each of the last two rows to have the add class as well.

[paulfitz]

Thanks for reporting this. I believe this got fixed a few days ago, in 63fa22a where I brought the styling logic used with handsontable into the main library. A quick test mimicking your example looks ok. Could you try again with an updated coopyhx.js?

[Floppy]

I've updated coopyhx, and I still see the same output on that row, so this hasn't been fixed as far as I can tell. The exact code I've got to produce this is:

<script>
  $(document).ready(function () {

    var old_table = new coopy.CoopyTableView([["planetary_body","acceleration"],["Earth","9.80665"],["Moon","1.625"],["Sun","274.1"],["Venus","8.872"],["Mars","3.78"],["Jupiter","25.93"],["Io","1.789"],["Europa","1.314"],["Ganymede","1.426"],["Callisto","1.24"],["Saturn","11.19"],["Titan","1.3455"],["Uranus","9.01"],["Titania","0.379"],["Oberon","0.347"],["Neptune","11.28"],["Triton"," 0.779"],["Pluto"," 0.61"]]);
    var new_table = new coopy.CoopyTableView([["planetary_body","aphelion","acceleration"],["Earth","152098232","9.80665"],["Moon",null,"1.625"],["Sun","0","274.1"],["Mercury","69816900","3.7"],["Venus","108939000","8.872"],["Mars","249209300","3.78"],["Jupiter","816520800","25.93"],["Io",null,"1.789"],["Europa",null,"1.314"],["Ganymede",null,"1.426"],["Callisto",null,"1.24"],["Saturn","1513325783","11.19"],["Titan",null,"1.3455"],["Uranus","3004419704","9.01"],["Titania",null,"0.379"],["Oberon",null,"0.347"],["Neptune","4553946490","11.28"],["Triton",null,"0.779"],["Pluto","7311000000","0.61"]]);

    var alignment = coopy.compareTables(old_table,new_table).align();

    var data_diff = [];      
    var table_diff = new coopy.CoopyTableView(data_diff);

    var flags = new coopy.CompareFlags();
    var highlighter = new coopy.TableDiff(alignment,flags);
    highlighter.hilite(table_diff);

    var diff2html = new coopy.DiffRender();
    diff2html.render(table_diff);
    diff_html = diff2html.html()

    $("#csv-diff-4edd469d88024ef0de2995d14419218a").html(diff_html);

  });
</script>
<div class='csvdiff highlighter tabular ' id='csv-diff-4edd469d88024ef0de2995d14419218a'></div>

[paulfitz]

Thanks for the test code. May I add it to the coopyhx tests, with you as author and the license as MIT?

Strange, I'm seeing:

<tr class="modify"><td class="modify">→</td><td>Triton</td><td class="add"></td><td class="modify"> 0.779→0.779</td></tr>
<tr class="modify"><td class="modify">→</td><td>Pluto</td><td class="add">7311000000</td><td class="modify"> 0.61→0.61</td></tr>

on a local test. You are missing the class="add" parts, is that correct? Is there any possibility we've hit browser caching anywhere along the line?

Just as a cross-check, could you try installing command-line coopyhx with npm (npm install -g coopyhx or similar), and then (with VERSION1.csv and VERSION2.csv being the two table versions) do:

coopyhx diff VERSION1.csv VERSION2.csv --output diff.csv
coopyhx render --fragment diff.csv

Check the last two lines for class="add" in them. If it looks good, then /usr/lib/node_modules/coopyhx/coopyhx.js should be what you want, and I need to pick a better way to publish that js.

Sorry you are hitting trouble, and thanks for being one of coopyhx's first users :-)

[Floppy]

Yes, adding it to test suite is fine. And yes, I'm missing the class='add' parts. I've just tried 3 different browsers, to the same effect, so it's not a caching thing.

However, trying it through node worked, and copying the coopyhx.js from the npm install did show up a load of other changes. Now that version is in my gitlab source, it works great! Perhaps I had picked up another older version somehow.

Anyway, working now:

There is a small error in the last line, but the HTML is correct, so that's my CSS problem. Thanks!

[paulfitz]

Right, on the last line there is now a tr class="modify", which wasn't there before, to allow overall row styling. So to keep the old styling the blue/modify colour needs to be restricted to td.modify. I'll avoid changing this stuff in future now that the library is in use by at least one other person :-).

By the way, I feel like the way removal of a space in a cell is being rendered by coopyhx isn't super helpful (the "0.779->0.779"), maybe I should add quotes in that case? Or something?

[Floppy]

Yep, I've changed the CSS to handle that now.

As for the space, yeah, it's a tricky one. It's there in the generated code, of course, but the browser is ignoring it. Perhaps spaces should be encoded to   or something, though it would still be rather subtle in this case.