You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by gs...@apache.org on 2023/12/15 09:44:03 UTC
svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Author: gstein
Date: Fri Dec 15 09:44:03 2023
New Revision: 1914679
URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
Log:
class DifflibDiffContent does not work, and maybe never did. There is
no .next() method on the unified_diff() result object. While it would
be possible to use the next() function, it is better to just remove
this entirely and require the host OS to have a diff executable (not a
hard requirement).
* tools/hook-scripts/mailer/mailer.py:
(DiffGenerator.__getitem__): remove OSError exception, as the diff
executable must be present.
(class DifflibDiffContent): removed.
Modified:
subversion/trunk/tools/hook-scripts/mailer/mailer.py
Modified: subversion/trunk/tools/hook-scripts/mailer/mailer.py
URL: http://svn.apache.org/viewvc/subversion/trunk/tools/hook-scripts/mailer/mailer.py?rev=1914679&r1=1914678&r2=1914679&view=diff
==============================================================================
--- subversion/trunk/tools/hook-scripts/mailer/mailer.py (original)
+++ subversion/trunk/tools/hook-scripts/mailer/mailer.py Fri Dec 15 09:44:03 2023
@@ -1016,17 +1016,13 @@ class DiffGenerator:
if binary:
content = src_fname = dst_fname = None
else:
- src_fname, dst_fname = diff.get_files()
- try:
+ src_fname, dst_fname = diff.get_files()
content = DiffContent(self.cfg.get_diff_cmd(self.group, {
'label_from' : label1,
'label_to' : label2,
'from' : src_fname,
'to' : dst_fname,
}))
- except OSError:
- # diff command does not exist, try difflib.unified_diff()
- content = DifflibDiffContent(label1, label2, src_fname, dst_fname)
# return a data item for this diff
return _data(
@@ -1101,36 +1097,6 @@ class DiffContent:
raise IndexError
line, ltype, self.seen_change = _classify_diff_line(line, self.seen_change)
- return _data(
- raw=line,
- text=line[1:-1], # remove indicator and newline
- type=ltype,
- )
-
-
-class DifflibDiffContent():
- "This is a generator-like object returning annotated lines of a diff."
-
- def __init__(self, label_from, label_to, from_file, to_file):
- import difflib
- self.seen_change = False
- fromlines = open(from_file, 'U').readlines()
- tolines = open(to_file, 'U').readlines()
- self.diff = difflib.unified_diff(fromlines, tolines,
- label_from, label_to)
-
- def __nonzero__(self):
- # we always have some items
- return True
-
- def __getitem__(self, idx):
-
- try:
- line = self.diff.next()
- except StopIteration:
- raise IndexError
-
- line, ltype, self.seen_change = _classify_diff_line(line, self.seen_change)
return _data(
raw=line,
text=line[1:-1], # remove indicator and newline
Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 15 dec. 2023 kl 10:45 skrev <gs...@apache.org>:
> Author: gstein
> Date: Fri Dec 15 09:44:03 2023
> New Revision: 1914679
>
> URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
> Log:
> class DifflibDiffContent does not work, and maybe never did. There is
> no .next() method on the unified_diff() result object. While it would
> be possible to use the next() function, it is better to just remove
> this entirely and require the host OS to have a diff executable (not a
> hard requirement).
>
I don't completely understand the technical details, but the Swig bindings
([1], lines 201 and below) use svn.diff.file_diff_2 to create a diff.
Would it make sense to use this instead of a hard requirement on a diff
executable?
Kind regards,
Daniel
[1] subversion/bindings/swig/python/svn/fs.py
>
> * tools/hook-scripts/mailer/mailer.py:
> (DiffGenerator.__getitem__): remove OSError exception, as the diff
> executable must be present.
> (class DifflibDiffContent): removed.
>
> Modified:
> subversion/trunk/tools/hook-scripts/mailer/mailer.py
>
> Modified: subversion/trunk/tools/hook-scripts/mailer/mailer.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/tools/hook-scripts/mailer/mailer.py?rev=1914679&r1=1914678&r2=1914679&view=diff
>
> ==============================================================================
> --- subversion/trunk/tools/hook-scripts/mailer/mailer.py (original)
> +++ subversion/trunk/tools/hook-scripts/mailer/mailer.py Fri Dec 15
> 09:44:03 2023
> @@ -1016,17 +1016,13 @@ class DiffGenerator:
> if binary:
> content = src_fname = dst_fname = None
> else:
> - src_fname, dst_fname = diff.get_files()
> - try:
> + src_fname, dst_fname = diff.get_files()
> content = DiffContent(self.cfg.get_diff_cmd(self.group, {
> 'label_from' : label1,
> 'label_to' : label2,
> 'from' : src_fname,
> 'to' : dst_fname,
> }))
> - except OSError:
> - # diff command does not exist, try difflib.unified_diff()
> - content = DifflibDiffContent(label1, label2, src_fname,
> dst_fname)
>
> # return a data item for this diff
> return _data(
> @@ -1101,36 +1097,6 @@ class DiffContent:
> raise IndexError
>
> line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
> - return _data(
> - raw=line,
> - text=line[1:-1], # remove indicator and newline
> - type=ltype,
> - )
> -
> -
> -class DifflibDiffContent():
> - "This is a generator-like object returning annotated lines of a diff."
> -
> - def __init__(self, label_from, label_to, from_file, to_file):
> - import difflib
> - self.seen_change = False
> - fromlines = open(from_file, 'U').readlines()
> - tolines = open(to_file, 'U').readlines()
> - self.diff = difflib.unified_diff(fromlines, tolines,
> - label_from, label_to)
> -
> - def __nonzero__(self):
> - # we always have some items
> - return True
> -
> - def __getitem__(self, idx):
> -
> - try:
> - line = self.diff.next()
> - except StopIteration:
> - raise IndexError
> -
> - line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
> return _data(
> raw=line,
> text=line[1:-1], # remove indicator and newline
>
>
>
Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Posted by Greg Stein <gs...@gmail.com>.
On Fri, Dec 15, 2023 at 4:51 AM Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
wrote:
> Hi,
>
> On 2023/12/15 18:44, gstein@apache.org wrote:
> > Author: gstein
> > Date: Fri Dec 15 09:44:03 2023
> > New Revision: 1914679
> >
> > URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
> > Log:
> > class DifflibDiffContent does not work, and maybe never did. There is
> > no .next() method on the unified_diff() result object. While it would
> > be possible to use the next() function, it is better to just remove
> > this entirely and require the host OS to have a diff executable (not a
> > hard requirement).
>
> I don't want to make objection to removing DifflibDiffContent, but
> it seems it worked on Python 2, where generator object has next()
> method instead of .__next__() method. So it is a missed feature
> while porting Python 3.
>
We could probably use the next() builtin function (available in Py2 and
Py3).
But should we? Is there a platform without a diff executable? Maybe Windows?
I found this "use difflib" feature was added in 2010, in r1032568, with no
further work in the intervening years. How many people use this?
Consider: it didn't work in Py3. "All" unix-ish systems have "diff". So
we're talking about (maybe?) Windows-based svn administrators using Python
2.
Personally, I think it is safe to just omit this "feature", which is likely
unused today.
Cheers,
-g
Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
Hi,
On 2023/12/15 18:44, gstein@apache.org wrote:
> Author: gstein
> Date: Fri Dec 15 09:44:03 2023
> New Revision: 1914679
>
> URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
> Log:
> class DifflibDiffContent does not work, and maybe never did. There is
> no .next() method on the unified_diff() result object. While it would
> be possible to use the next() function, it is better to just remove
> this entirely and require the host OS to have a diff executable (not a
> hard requirement).
I don't want to make objection to removing DifflibDiffContent, but
it seems it worked on Python 2, where generator object has next()
method instead of .__next__() method. So it is a missed feature
while porting Python 3.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>
Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Posted by Daniel Sahlberg <da...@gmail.com>.
Den fre 15 dec. 2023 kl 10:45 skrev <gs...@apache.org>:
> Author: gstein
> Date: Fri Dec 15 09:44:03 2023
> New Revision: 1914679
>
> URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
> Log:
> class DifflibDiffContent does not work, and maybe never did. There is
> no .next() method on the unified_diff() result object. While it would
> be possible to use the next() function, it is better to just remove
> this entirely and require the host OS to have a diff executable (not a
> hard requirement).
>
I don't completely understand the technical details, but the Swig bindings
([1], lines 201 and below) use svn.diff.file_diff_2 to create a diff.
Would it make sense to use this instead of a hard requirement on a diff
executable?
Kind regards,
Daniel
[1] subversion/bindings/swig/python/svn/fs.py
>
> * tools/hook-scripts/mailer/mailer.py:
> (DiffGenerator.__getitem__): remove OSError exception, as the diff
> executable must be present.
> (class DifflibDiffContent): removed.
>
> Modified:
> subversion/trunk/tools/hook-scripts/mailer/mailer.py
>
> Modified: subversion/trunk/tools/hook-scripts/mailer/mailer.py
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/tools/hook-scripts/mailer/mailer.py?rev=1914679&r1=1914678&r2=1914679&view=diff
>
> ==============================================================================
> --- subversion/trunk/tools/hook-scripts/mailer/mailer.py (original)
> +++ subversion/trunk/tools/hook-scripts/mailer/mailer.py Fri Dec 15
> 09:44:03 2023
> @@ -1016,17 +1016,13 @@ class DiffGenerator:
> if binary:
> content = src_fname = dst_fname = None
> else:
> - src_fname, dst_fname = diff.get_files()
> - try:
> + src_fname, dst_fname = diff.get_files()
> content = DiffContent(self.cfg.get_diff_cmd(self.group, {
> 'label_from' : label1,
> 'label_to' : label2,
> 'from' : src_fname,
> 'to' : dst_fname,
> }))
> - except OSError:
> - # diff command does not exist, try difflib.unified_diff()
> - content = DifflibDiffContent(label1, label2, src_fname,
> dst_fname)
>
> # return a data item for this diff
> return _data(
> @@ -1101,36 +1097,6 @@ class DiffContent:
> raise IndexError
>
> line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
> - return _data(
> - raw=line,
> - text=line[1:-1], # remove indicator and newline
> - type=ltype,
> - )
> -
> -
> -class DifflibDiffContent():
> - "This is a generator-like object returning annotated lines of a diff."
> -
> - def __init__(self, label_from, label_to, from_file, to_file):
> - import difflib
> - self.seen_change = False
> - fromlines = open(from_file, 'U').readlines()
> - tolines = open(to_file, 'U').readlines()
> - self.diff = difflib.unified_diff(fromlines, tolines,
> - label_from, label_to)
> -
> - def __nonzero__(self):
> - # we always have some items
> - return True
> -
> - def __getitem__(self, idx):
> -
> - try:
> - line = self.diff.next()
> - except StopIteration:
> - raise IndexError
> -
> - line, ltype, self.seen_change = _classify_diff_line(line,
> self.seen_change)
> return _data(
> raw=line,
> text=line[1:-1], # remove indicator and newline
>
>
>
Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py
Posted by Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>.
Hi,
On 2023/12/15 18:44, gstein@apache.org wrote:
> Author: gstein
> Date: Fri Dec 15 09:44:03 2023
> New Revision: 1914679
>
> URL: http://svn.apache.org/viewvc?rev=1914679&view=rev
> Log:
> class DifflibDiffContent does not work, and maybe never did. There is
> no .next() method on the unified_diff() result object. While it would
> be possible to use the next() function, it is better to just remove
> this entirely and require the host OS to have a diff executable (not a
> hard requirement).
I don't want to make objection to removing DifflibDiffContent, but
it seems it worked on Python 2, where generator object has next()
method instead of .__next__() method. So it is a missed feature
while porting Python 3.
Cheers,
--
Yasuhito FUTATSUKI <fu...@yf.bsdclub.org>