Make difflib._mdiff() a standard method

Ideas
1

I found that using _mdiff() indifflib to be useful when I have to crate a HTML table different to what make_table() generates.
Any reasons not to make that a standard method?

2

I suggest filing a feature request for it to be promoted to a public API. Inertia of the existing code remaining as it is (it is stable and doesn’t change often) is likely the main reason it hasn’t.

1 Like
3

I guess one reason is that its output isn’t a very good public API.

Generally, having to re-parse serialized output (from ndiff) suggests that either we have inadequate/missing lower-level API (that would deal with iterators of (named)tuples/dataclasses, rather than embedded \n+ or \0+ markers), or that the function predates that API.

But, we already have _mdiff, and I don’t think anyone’s eager to add a new differ whose output wouldn’t require parsing. Documenting _mdiff’s output and exposing it might be the most pragmatic thing to do.

2 Likes
4

May be make_table() should be expanded so that it allows some customization to HTML?
At the moment make_table() method makes the HTML design designs for the user of the API.

5

(Resurrecting the thread, as I’m currently chatting to @kesara about this at the PyCon AU sprints).

Looking at the way make_table is implemented, the row rendering part of the method is:

        s = []
        fmt = '            <tr><td class="diff_next"%s>%s</td>%s' + \
              '<td class="diff_next">%s</td>%s</tr>\n'
        for i in range(len(flaglist)):
            if flaglist[i] is None:
                # mdiff yields None on separator lines skip the bogus ones
                # generated for the first line
                if i > 0:
                    s.append('        </tbody>        \n        <tbody>\n')
            else:
                s.append( fmt % (next_id[i],next_href[i],fromlist[i],
                                           next_href[i],tolist[i]))
        if fromdesc or todesc:
            header_row = '<thead><tr>%s%s%s%s</tr></thead>' % (
                '<th class="diff_next"><br /></th>',
                '<th colspan="2" class="diff_header">%s</th>' % fromdesc,
                '<th class="diff_next"><br /></th>',
                '<th colspan="2" class="diff_header">%s</th>' % todesc)
        else:
            header_row = ''

        table = self._table_template % dict(
            data_rows=''.join(s),
            header_row=header_row,
            prefix=self._prefix[1])

        return table.replace('\0+','<span class="diff_add">'). \
                     replace('\0-','<span class="diff_sub">'). \
                     replace('\0^','<span class="diff_chg">'). \
                     replace('\1','</span>'). \
                     replace('\t','&nbsp;')

The prefix is primarily just a sequential counter managed by the class to ensure HTML anchors for multiple tables in the same page are distinct.

So one possibility for expanding make_table to allow rendering customisation would be to split out a make_table_parts helper that emits a tuple of the pieces the rendering code needs.

The key challenge with doing that would be around separating the parsing and formatting steps for the table rows, as those are currently tightly intertwined:

  • the line numbers + each side of the line diffs are rendered to HTML when collecting lines to include in the table
  • the flag list processing defines assorted backwards-and-forwards links (as well as handling files with no changes and files with no contents)
  • the side by side table cell rendering happens in the snippet quoted above
  • the file section separator rendering happens in the snippet quoted above
  • the header row rendering happens in the snippet quoted above
  • the conversion of the mdiff \0 and \1 markers to HTML spans happens as a text post-processing step
6

@kesara Looking into the idea of “configuration over customisation”, some possibilities that come to mind:

  • in make_table (or the HtmlDiff constructor?), accept a cell_config dict with cellspacing and cellpadding keys (attributes to be set on the table definition)
  • in make_file (or the HtmlDiff constructor?), accept a fonts list (with a DEFAULT_FONTS class variable)
  • in make_file (or the HtmlDiff constructor?), accept a colors dict (with a DEFAULT_COLORS class variable). Keys would be header_bg, next_bg, add_bg, chg_bg, sub_bg (matching the diff_* HTML class names, with a _bg suffix to allow for potential future _fg customisation)

The advantage I see to doing things that way over accepting arbitrary CSS styling is that it avoids locking in the exact class names as part of the public API (although given the ability to use make_table to inject diff tables into arbitrary pages, it could be argued that we’ve effectively already done that).

7

@ncoghlan I like the “configuration over customisation” idea.

I think that should allow ability to replace the head of generated HTML.

Also at the moment make_table adds a legend table, I think that should be able to configured too. ie. enable, disable, placement of legend and content of it.

8

Adding the legend is part of make_file. It uses the same text background styles as make_table.

We could theoretically expose make_styles and make_legend separately (just as make_table is exposed separately), for plugging into custom HTML pages.

1 Like
9

There are others who make use of _mdiff() method as well.[1]
I think rather than making the make_table() method more complex, their might be value to promoting the _mdiff() method, which is an easy win.

  1. Promote difflib._mdiff() to public API · Issue #129922 · python/cpython · GitHub ↩︎