[Patches] [ python-Patches-914575 ] difflib side by side diff
support, diff.py s/b/s HTML option
SourceForge.net
noreply at sourceforge.net
Tue Jul 27 16:49:23 CEST 2004
Patches item #914575, was opened at 2004-03-11 17:50
Message generated for change (Comment added) made by dmgass
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=914575&group_id=5470
Category: Modules
Group: Python 2.4
Status: Open
Resolution: None
Priority: 6
Submitted By: Dan Gass (dmgass)
Assigned to: Nobody/Anonymous (nobody)
Summary: difflib side by side diff support, diff.py s/b/s HTML option
Initial Comment:
lib/difflib.py:
Added support for generating side by side differences.
Intended to be used for generating HTML pages but is
generic where it can be used for other types of markup.
tools/scripts/diff.py:
Added -m option to use above patch to generate an
HTML page of side by side differences between two
files. The existing -c option when used with the new -m
option controls whether contextual differences are
shown or whether the entire file is displayed (number of
context lines is controlled by existing -l option).
NOTES:
(1) Textual context diffs were included as requested. In
addition, full and contextual HTML side by side
differences (that this patch generated) are also included
to assist in analyzing the differences and showing an
example.
(2) If this functionality is worthy of inclusion in the
standard python distribution, I am willing to provide more
documentation or make modifications to change the
interface or make improvements.
(3) When using Internet Explorer some font sizes seem
to skew things a bit. Generally I've found the "smallest"
to work best. If someone knows why, I'd be interested
in making any necessary adjustments in the generated
HTML.
----------------------------------------------------------------------
>Comment By: Dan Gass (dmgass)
Date: 2004-07-27 09:49
Message:
Logged In: YES
user_id=995755
I am considering making an optional argument to the
constructor for specifying tab indentation. If nothing was
passed it defaults to not handling tabs (better
performance). If a number is passed, the string sequences
(lists of lines) would be wrapped with a generator function
to convert the tabs in each line with the expandtabs string
method. The other option is to expand tabs all the time.
If one is going to handle tabs it must be done on the input
because once it is processed (markup added) the algorithm
for expanding tabs becomes really compilicated. Any
opinions regarding these options are welcome.
I think I should change the default prefix (_default_prefix)
to be a class member rather than initialize it with the
constructor. (The default prefix is used to generate unique
anchor names so there are no conflicts between multiple
tables on the same HTML page). I'm leaning this way because
a user may create separate instances of HtmlDiff (to have
different ndiff or tab settings) but place the tables on
the same page. If I left it the hyperlinks in the second
table would jump to the first table.
----------------------------------------------------------------------
Comment By: Jim Jewett (jimjjewett)
Date: 2004-07-27 09:11
Message:
Logged In: YES
user_id=764593
Technically, HTML treats all whitespace (space, tab, newline,
carriage return, etc) as interchangable with a single space,
except sometimes when you are in a <pre> block.
In practice, user agents often honor it anyhow. If tab isn't
working, you might have better luck replacing it with a series
of 8 , ((ampersand, then "nbsp", then semicolon)*8) but
I'm not sure the ugliness is worth it.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-26 01:34
Message:
Logged In: YES
user_id=995755
Updated the patch as follows:
1) handles no differences now in both full and context mode
(Adam discovered that it crashed when no differences in
context mode).
2) handles empty string lists (I'm pretty sure it would have
crashed had someone tried it).
3) "links" column in the middle now appears on the left as well.
4) Moved prefix argument from constructor to make_file and
make_table methods. Also made it work by default so that if
you are generating multiple tables and putting them on the
same HTML page there are no anchor name conflicts.
5) mdiff() function now is protected: _mdiff() so we are at
liberty to change the interface in the future
6) templates moved to protected global variables (still public
members of the class) so that the indenting could be
improved.
7) Improved the indenting in other areas so that the HTML is
now much more readable.
8) Inlined the special character escaping so that the xml.sax
library function is not used (this seems to have improved the
performance quite a bit).
9) Moved as many <table> attributes as possible to a style
sheet class. Adam, please review this incase I missed some.
10) Expanded test suite to cover the above changes and
made it easier to baseline.
11) Updated documentation to reflect above changes.
NOTES
N1) Raymond, you had mentioned this crashes when the
newlines are stripped. I modified the test to include stripping
and not and have found both to work without having to fix
anything. Can you duplicate what you saw and give me more
info?
N2) I've noticed the HTML does not render tabs very well (at
all). Is this OK or does anyone have any good ideas?
N3) I've attached the patch (you will also need the new file
test_difflib_expect.html). I've zipped the patched code as
well as example side by side differences of the patch. Diffs
ending with _UPDATE.html show differences between the
patched code and the last version of the patched code (what
I've done since my last posting). Diffs ending with
_PATCH.html show differences between the patched code
and what I obtained from CVS a couple weeks back:
python/python/dist/src/Lib/difflib.py -- rev 1.21
python/python/dist/src/Tools/scripts/diff.py -- rev 1.2
python/python/dist/src/Lib/test/test_difflib.py -- rev 1.10
python/python/dist/src/Doc/lib/libdifflib.tex -- rev 1.17
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-25 08:51
Message:
Logged In: YES
user_id=995755
Adam's suggestion seems to make alot of sense regarding
moving the table attributes to a class. I will implement it in
the next update. I will experiment with the font issue but will
likely hold off on implementing anything.
Adam -- thanks for the quick feedback.
----------------------------------------------------------------------
Comment By: Adam Souzis (asouzis)
Date: 2004-07-25 01:51
Message:
Logged In: YES
user_id=57486
> Why move the attributes to a class for the two tables?
In general its good practice to separate the style info from
the html. For this case in particular i had to do this
because i'm embedding the diff tables in a generic page
template that applies general style rules to tables in the
page. By adding a class for those tables i was able to add a
rule for it that overrode the more general table rule.
> Can I get a recommended solution for the font issue?
Not sure, I added this rule:
.diff_table th, .diff_table td
{ font-size: smaller; }
But its a bit problematic as Mozilla defaults to a
considerably smaller monotype font size than IE so its hard
to be consistent across browsers.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-24 10:29
Message:
Logged In: YES
user_id=995755
I will be implementing the following suggestions:
Raymond 7/19 & 7/23 [all]
Adam 7/17 [all]
Adam 7/23 [partial, but I need feedback]
- Can I get a recommended solution for the font issue?
- I'll likely do the additional links column on the left
- Why move the attributes to a class for the two tables?
Unless the template issue (public/protected/private) is
discussed further, I will assume everyone is in agreement or
can live with Raymond's suggestion & clarification.
I will not be implementing any changes to the API right now.
I lean somewhat strong toward leaving the optional
arguments as they are but this can be talked about further if
anyone thinks there are strong arguments to the contrary.
Thanks Raymond, Adam, Jim, and Neal for your latest
comments. Unless I get further suggestions, expect the
patch hopefully by the end of the weekend.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2004-07-23 23:20
Message:
Logged In: YES
user_id=80475
To clarify, the private variables are just a way to move the
defaults out of the class definition and give the OP more
control over formatting:
_legend = """
<table summary="Legends" style="font-family:Courier">
<tr> <th colspan="2"> Legends </th> </tr>
<tr> <td> <table border="" summary="Colors">
<tr><th> Colors </th> </tr>
<tr><td
class="diff_add"> Added </td></tr>
<tr><td class="diff_chg">Changed</td> </tr>
<tr><td class="diff_sub">Deleted</td> </tr>
</table></td>
<td> <table border="" summary="Links">
<tr><th colspan="2"> Links </th> </tr>
<tr><td>(f)irst change</td> </tr>
<tr><td>(n)ext change</td> </tr>
<tr><td>(t)op</td> </tr>
</table></td> </tr>
</table>
"""
class HtmlDiff(object):
file_template = _file_template
styles = _styles
linenum_template = _linenum_template
table_template = _table_template
legend = _legend
def __init__(self,prefix=['from','to'], linejunk=None,
. . .
----------------------------------------------------------------------
Comment By: Adam Souzis (asouzis)
Date: 2004-07-23 15:01
Message:
Logged In: YES
user_id=57486
certainly you need to be able to access the templates and
modify them. And should be documented too -- one of the
first things i wanted to know is how can i customize the
output. But a config object seems like conceptual clutter.
keyword arguments can be ignored and are just as persistent
if you use ** on a dict.
few more suggestions:
* the default font seems too large on both ie 6 and firefox
(this is with 1200 x 800 screen resolution, so it'd be even
larger at a lower resolution).
* maybe the links column (t, f, n) should also be on the
left -- often there are long lines and you need to scroll
over to access it.
* add class attributes to the two tables used in the
templates and move their styles to there (except i believe
that IE doesn't honor cellpadding/spacing styles in css)
thanks
----------------------------------------------------------------------
Comment By: Jim Jewett (jimjjewett)
Date: 2004-07-23 11:31
Message:
Logged In: YES
user_id=764593
Actually, I'm not convinced that the templates should be
private, though I can certainly see protected. (double-
underscore private is mangled; single-underscore protected will
be left out of some imports and doco, but acts like a regular
variable if people choose to use it anyway.)
I'm still thinking about nnorwitz's suggestion; the config option
could be ignored by most people ("use the defaults") but would
hold persistent state for people with explicit preferences (since
they'll probably want to make the same changes to all
compares).
----------------------------------------------------------------------
Comment By: Neal Norwitz (nnorwitz)
Date: 2004-07-23 11:10
Message:
Logged In: YES
user_id=33168
I have not reviewed the code yet, so this is just a general
comment. I don't know if it is applicable.
You could create a configuration class that has attributes.
A user would only need to assign to the params that they
want to update. If you change the names you could add
properties to deal with backwards compatibility.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-23 10:48
Message:
Logged In: YES
user_id=995755
I agree the API for make_file() and make_table() has alot of
optional parameters that can look daunting. The alternative
of providing accessor methods is less appealing to me. It
sounds like Jim & I are in agreement on this.
As far as the compromise of making the templates private
members, I am assuming Jim is in favor of it. I'm in favor
of them being members, it doesn't matter to me if they are
private. I'll wait until Raymond weighs in on this one
before I move on it.
Jim -- Thanks for your input.
----------------------------------------------------------------------
Comment By: Jim Jewett (jimjjewett)
Date: 2004-07-23 10:17
Message:
Logged In: YES
user_id=764593
For a context or unified diff, you don't really need to
parametrize it very much. Having a much larger API for side-
by-side puts things out of balance, which can make side-by-
side look either more difficult or preferred.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-23 08:57
Message:
Logged In: YES
user_id=995755
Maybe a good compromise would be to leave them members of
the class but name them to imply they are private (add a
leading underscore). That way if someone wants to create
their own subclass they can at their own risk. They can
always write a little extra logic to insure the interface
didn't change and raise a custom exception explaining what
happened and what they need to look at.
I didn't read (2) in Raymond's comments. Could you expand
on those concerns?
----------------------------------------------------------------------
Comment By: Jim Jewett (jimjjewett)
Date: 2004-07-23 08:24
Message:
Logged In: YES
user_id=764593
I agree that users should be able to override the template.
I think Raymond was concerned about
(1) backwards compatibility if you want to change the
interface. For instance, you might later decide that there
should be separate templates for header/footer/match section/
changed line/added line/deleted line.
(2) matching the other diff options, so this doesn't look far
more complicated.
Unfortunately, at the moment I don't see a good way to solve
those; using a private member and access functions wouldn't
really simplify things much.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-23 06:53
Message:
Logged In: YES
user_id=995755
I intend on implementing all the suggestions from the last
two comment submissions when I hear back from Raymond
Hettinger. I'm questioning making the templates private
global variables. I intentionally made them members of a
class so that they could be overridden when the class is
subclassed.
----------------------------------------------------------------------
Comment By: Raymond Hettinger (rhettinger)
Date: 2004-07-19 08:12
Message:
Logged In: YES
user_id=80475
Unfortunately, I do not have time to give this more review.
Several thoughts:
- consider making mdiff as private. that will leave its API
flexible to accomodate future changes
- move the templates to private global variables and work to
improve their indentation so that the html is readable
- inline the code for _escape from sax. the three replaces
are not worth the interdependency
- the methods work fine with properly formatted input but
crash badly when the newlines have been stripped. So,
either make the code more flexible or add error handling.
- overall, nice job.
----------------------------------------------------------------------
Comment By: Adam Souzis (asouzis)
Date: 2004-07-17 14:12
Message:
Logged In: YES
user_id=57486
The diffs look cool but if the to and from lines are
identical an exception is thrown:
File "<string>", line 23, in makeDiff
File "c:\_dev\rx4rdf\rx\htmldiff.py", line 1687, in make_file
summary=summary))
File "c:\_dev\rx4rdf\rx\htmldiff.py", line 1741, in make_table
if not diff_flags[0]:
IndexError: list index out of range
(This is on python 2.3.3 -- I renamed your difflib.py to
htmldiff.py and put it in site-packages)
Perhaps you should add the case of two identical files to
test_difflib.py.
thanks
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-16 04:16
Message:
Logged In: YES
user_id=995755
Since I have not gotten any feedback on the user interface I
took the liberty to tweek it the best I thought how. Since I
consider this new functionality very solid I went ahead and
created changes to the documentation and test suite.
Per Martin v. Löwis (loewis) instructions I generated a patch
which hopefully meets his needs. My CVS access is limited to
the web interface and thus the patch is based on the latest
checked in as of today:
python/python/dist/src/Lib/difflib.py -- rev 1.21
python/python/dist/src/Tools/scripts/diff.py -- rev 1.2
python/python/dist/src/Lib/test/test_difflib.py -- rev 1.10
python/python/dist/src/Doc/lib/libdifflib.tex -- rev 1.17
Note, I am not very familiar with .tex but it seemed straight
forward. I editted the file by hand and it should be very close
to what I intended. Unfortunately I am not set up to convert
the .tex to HTML. I may try that next week.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-16 04:13
Message:
Logged In: YES
user_id=995755
Since I have not gotten any feedback on the user interface I
took the liberty to tweek it the best I thought how. Since I
consider this new functionality very solid I went ahead and
created changes to the documentation and test suite.
Per Martin v. Löwis (loewis) instructions I generated a patch
which hopefully meets his needs. My CVS access is limited to
the web interface and thus the patch is based on the latest
checked in as of today:
python/python/dist/src/Lib/difflib.py -- rev 1.21
python/python/dist/src/Tools/scripts/diff.py -- rev 1.2
python/python/dist/src/Lib/test/test_difflib.py -- rev 1.10
python/python/dist/src/Doc/lib/libdifflib.tex -- rev 1.17
Note, I am not very familiar with .tex but it seemed straight
forward. I editted the file by hand and it should be very close
to what I intended. Unfortunately I am not set up to convert
the .tex to HTML. I may try that next week.
----------------------------------------------------------------------
Comment By: Martin v. Löwis (loewis)
Date: 2004-07-15 01:30
Message:
Logged In: YES
user_id=21627
The pack lacks changes to the documentation (libdifflib.tex)
and changes to the test suite. Please always submit patches
as single unified or context diffs, rather than zip files of
the revised files.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-07-02 13:17
Message:
Logged In: YES
user_id=995755
With the updated patch code I just posted, both full and
contextual differences pass the validator.w3.org check
(XHTML 1.0 Transitional). Also the extra carriage returns
put in by DOS were removed from the difflib.py patch.
----------------------------------------------------------------------
Comment By: Walter Dörwald (doerwalter)
Date: 2004-06-25 13:01
Message:
Logged In: YES
user_id=89016
Submitting difflib_context.html to validator.w3.org gives 2333
errors. The character encoding is missing and there's no
DOCTYPE declaration. The rest of the errors seem to be
mostly related to the nowrap attribute (which must be written
as nowrap="nowrap", as this is the only allowed value).
Furthermore the patch contains a mix of CR and CRLF
terminated lines.
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-06-24 01:23
Message:
Logged In: YES
user_id=995755
I just attached an updated patch. I based the patch on
diff.py(CVS1.1) and difflib.py(CVS1.20) which was the latest I
saw today on viewCVS.
The following enhancements were made:
1) user interface greatly simplified for generating HTML (see
diff.py for example)
2) generated HTML now 4.01 Transitional compliant (so says
HTML Tidy)
3) HTML color scheme for differences now matches that used
by viewCVS.
4) differences table now has a row for each line making the
HTML less susceptible to browser quirks.
5) took care of all issues to date enumerated here in this
patch.
It would be great if I could get some help on:
A) getting some JavaScript? written to be able to select and
cut text from a single column (right now text is selected from
the whole row including both "from" and "to" text and line
numbers.
B) solving the line width issue. Currently the "from" / "to"
column is as wide as the widest line. Any ideas on wrapping
or scrolling?
As of now the only feature I may want to add in the near
future is optional tab expansion.
Thanks to those that have commented here and emailed me
with suggestions and advice!
----------------------------------------------------------------------
Comment By: Jim Jewett (jimjjewett)
Date: 2004-06-11 09:35
Message:
Logged In: YES
user_id=764593
Thank you; I have often wished for side-by-side, but not quite
badly enough to write it.
That said, I would recommend some tweaks to the formatting.
"font" is deprecated; "span" would be better.
On my display, the "next" lines don't always seem to be
counted (except the first one), so that the anchors column is
not lined up with the others. (This would also be fixed by the
separate rows suggestion.)
Ideally, the line numbers would be in a separate column from
the code, to make cut'n'paste easier. (Then you could replace
font.num (or span.num) with td.linenum.)
Ideally, each change group should be a separate row in the
table. (I realize that this probably means a two-layer iterator,
so that the line breaks and blank lines can be inserted
correctly in the code columns.)
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-04-28 23:30
Message:
Logged In: YES
user_id=995755
Also, I will need to submit the fix for making it behave nice
when there are no differences!!
----------------------------------------------------------------------
Comment By: Dan Gass (dmgass)
Date: 2004-04-09 08:30
Message:
Logged In: YES
user_id=995755
In the time since submission I've found that the interface to
the chgFmt and lineFmt functions (arguments of mdiff) should
include both the line number and an indication of side
(from/to). The use for it I've found is for dropping anchors
into the generated markup that it can be hyperlinked from
elsewhere.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=305470&aid=914575&group_id=5470
More information about the Patches
mailing list