Re: svn commit: r1914679 - /subversion/trunk/tools/hook-scripts/mailer/mailer.py

2023-12-15 Thread Daniel Sahlberg
Den fre 15 dec. 2023 kl 10:45 skrev :

> 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

2023-12-15 Thread Yasuhito FUTATSUKI
Hi,

On 2023/12/15 18:44, gst...@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