difflib should separate filename from timestamp with tab

[techtonik]

BPO	7585
Nosy	@tim-one, @pitrou, @bitdancer, @briancurtin
Files	difflib.tab_separated_filename.diff example.py issue7585.difflib-tab-separator.diff: base patch issue7585.difflib-tab-separator-no-trail.diff: no trailing whitespace issue7585.difflib-tab-update-docs.diff: (optional) update docs issue7585.difflib-tab-updoc-iso8601.diff: (opt) alt docs update issue7585.difflib-tab-unittest.diff: with unittest

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/bitdancer'
closed_at = <Date 2010-04-12.16:59:46.784>
created_at = <Date 2009-12-27.20:57:36.639>
labels = ['type-bug', 'library']
title = 'difflib should separate filename from timestamp with tab'
updated_at = <Date 2010-05-13.16:24:26.892>
user = 'https://bugs.python.org/techtonik'

bugs.python.org fields:

activity = <Date 2010-05-13.16:24:26.892>
actor = 'techtonik'
assignee = 'r.david.murray'
closed = True
closed_date = <Date 2010-04-12.16:59:46.784>
closer = 'r.david.murray'
components = ['Library (Lib)']
creation = <Date 2009-12-27.20:57:36.639>
creator = 'techtonik'
dependencies = []
files = ['15682', '15691', '16730', '16731', '16733', '16734', '16797']
hgrepos = []
issue_num = 7585
keywords = ['patch']
message_count = 21.0
messages = ['96924', '96961', '96980', '96986', '96989', '97013', '97018', '97164', '97166', '99167', '102046', '102154', '102160', '102318', '102323', '102530', '102551', '102958', '103011', '103041', '105641']
nosy_count = 5.0
nosy_names = ['tim.peters', 'pitrou', 'techtonik', 'r.david.murray', 'brian.curtin']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue7585'
versions = ['Python 2.7', 'Python 3.2']

[techtonik]

The patch inserts \t character between filename and timestamp in unified
and context diff headers.

According to specification by Guido Van Rossum =)
http://www.artima.com/weblogs/viewpost.jsp?thread=164293

And de-facto output from various tools
http://code.google.com/p/python-patch/source/browse/#svn/trunk/doc

And the common sense
--- that it is easier to split this stuff
+++ than this one into filename + timestamp

--- diff.py Sun Dec 27 16:08:28 2009
+++ trunk/diff.py Sun Dec 27 15:46:58 2009

[briancurtin]

I don't think the conditional checks around the timestamps are
necessary. Couldn't you just put the \t directly in the string that gets
yielded? That way the chunk headers always follow the same format.

[techtonik]

Conditional checks are required to prevent leaving trailing whitespace in
filename when date component is not present. Such trailing whitespace may
confuse patch tools. [1]

[1] http://code.google.com/p/python-patch/issues/detail?id=2

[briancurtin]

Shouldn't the patch tool handle that? FWIW, both the "svn diff" and *nix
diff utilities produce headers with the parts split by a tab character.

Would the code in example.py work in your tool to handle both tabs and
spaces?

[techtonik]

Filenames may contain spaces too.

--- handle dis Sun Dec 27 16:08:28 2009
--- или вот это пон, дек 27 16:08:28 2009

The last line is space separated filename and date in Russian locale.
Patch tool should handle that, but as you may see it is not always
possible. That's why difflib modification with \t separator will greatly
improve interoperability of difflib patches regardless of timestamp
format.

Stripping trailing whitespace when there is no timestamp serves the same
purpose. We can assume that patch tools are perfect, but I wouldn't
write my tool if that was true, so its better to be friendly on difflib
side and generate the output that won't require more work than necessary
to use it.

[briancurtin]

I agree that splitting with a tab character is good, but I think it
should be split by a tab character in every case.

If the separator is different based on what data is provided, then it
complicates parsing and would have to be explained in documentation that
we provide different header formats.

[techtonik]

This patch makes sure filename and date split by tab in every case when
date is provided.

[pitrou]

Is there any reason why you changed some of the examples in the docstrings (e.g. 'Sat Jan 26 23:30:50 1991' -> '1991-01-26 23:30:50')?

[techtonik]

It is the same reason as for removing recommendation from docstring to generate timestamps in the format returned by time.ctime(). See issue bpo-7582

[techtonik]

The reason is to provide a good usage example.

[techtonik]

depends on issue bpo-7583

[techtonik]

Refreshed patches - feel free to apply in any order you want.

Convenience link for reviews:
http://codereview.appspot.com/809043/show

[techtonik]

These patches are incrementally add features. Feel free to apply one that suits most. Codereview provides more convenient way to review differences between these.

attaching optional patch that removes recommendation to use ctime format for timestamps

[bitdancer]

Your last patch looks the best to me. I agree both that a tab should not be emitted if there is no date (which is what git, for example, does), and that ISO 8601 timestamps should be promoted as the preferred format.

As you pointed out, bpo-7583 needs to be resolved before this can be applied.

In the meantime it would be nice to add an additional test for the no-tab-if-no-date case.

[bitdancer]

We could avoid the 7583 problem by making the doctests use NORMALIZE_WHITESPACE and moving the real *tests* into the unittests for the module. I think that would be a good thing to do anyway.

[techtonik]

Added unittest for tab with and without set filedate.
Removed bpo-7583 dependency with NORMALIZE_WHITESPACE.

[bitdancer]

Great, thanks. I'll check this in when the branch is unfrozen.

[bitdancer]

Applied to trunk in r8004 and py3k in r8006.

[techtonik]

r80004 and r80006 to be exact.

Great! Thanks! =)

[bitdancer]

Yes, thanks for the typo correction.

[techtonik]

tag:difflib