Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .github/workflows/main.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
name: CI
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was considering moving CI to actions, but maybe we can propose this change in a different PR?

Copy link
Author

@huichen-cs huichen-cs Oct 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi, @matiasb ,

Thanks.

My original intention was to use this to run tests for Python 2.7.

As related, my initial pull request failed the test for Python 2.7 because codecs.open does not have the newline parameter. In a commit that follows, I revise it to use io.open for Python 2.7, and all tests passed.

If you think these changes are OK, I will resubmit the proposals, perhaps, in two pull requests, one is to add the newline parameter, and the other to add the configuration file for Github actions.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A comment motivated by the issues related to Python 2.7. The package unidiff supports Python 2.7:

python-unidiff/setup.py

Lines 48 to 49 in 3353080

"Programming Language :: Python :: 2",
'Programming Language :: Python :: 2.7',

The final release of Python 2.7 was about 2 years ago (PEP 373). The final release of Python 3.6 is expected to be in about 2 months (PEP 494). So it seems that moving to code compatible with only Python 3 would not cause issues.

Supporting only Python 3 will enable using new syntax, for example formatted string literals, parenthesization within with statements, and structural pattern matching (when the supported Python versions will all be >= 3.10).

For the full new syntax of formatted string literals, Python >= 3.8 is required. Nonetheless, this would still mean that three Python versions are supported (3.8, 3.9, 3.10), which aligns with the extent of support for past Python versions in packages like numpy, scipy, and networkx.

(Sidenote: Python 3.9 is in the CI test environments:

but not in the Trove classifiers:

python-unidiff/setup.py

Lines 45 to 54 in 3353080

classifiers=[
'Intended Audience :: Developers',
'Development Status :: 4 - Beta',
"Programming Language :: Python :: 2",
'Programming Language :: Python :: 2.7',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
],

)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should be ok dropping the Python 2 support. Will do the paperwork (so feel free to not keep things py2 compatible). And right, it seems we need to update the metadata. Thanks 👍


on:
push:
branches: [ master ]
pull_request:
branches: [ master ]

workflow_dispatch:

jobs:
build:
runs-on: self-hosted
strategy:
matrix:
python-version: [ '2.7', '3.6', '3.7', '3.8', '3.9' ]
fail-fast: false

steps:
- uses: actions/checkout@v2
- uses: actions/setup-python@v2
with:
python-version: ${{ matrix.python-version }}
architecture: 'x64'
- name: 'Run Tests'
run: $GITHUB_WORKSPACE/run_tests.sh
shell: bash

9 changes: 9 additions & 0 deletions tests/samples/git_cr.diff
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
diff --git a/src/test/org/apache/commons/math/util/ExpandableDoubleArrayTest.java b/src/test/org/apache/commons/math/util/ExpandableDoubleArrayTest.java
new file mode 100644
index 000000000..2b38fa232
--- /dev/null
+++ b/a.txt
@@ -0,0 +1,3 @@
+ "This line is broken into two lines by CR. " + "but it should be treated as one line in the text diff file"
+ "This has no CR"
+ "This line also has CR. " + "but it should also be treated as one line in the text diff file".
Expand Down
12 changes: 12 additions & 0 deletions tests/test_parser.py
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,18 @@ def test_parse_malformed_diff_shorter_than_expected(self):
with open(utf8_file, 'r') as diff_file:
self.assertRaises(UnidiffParseError, PatchSet, diff_file)

def test_from_filename_with_cr_in_diff_text_files(self):
"""Parse git diff text files that contain CR"""
utf8_file = os.path.join(self.samples_dir, 'samples/git_cr.diff')
self.assertRaises(UnidiffParseError, PatchSet.from_filename, utf8_file)

ps1 = PatchSet.from_filename(utf8_file, newline='\n')
import io
with io.open(utf8_file, 'r', newline='\n') as diff_file:
ps2 = PatchSet(diff_file)

self.assertEqual(ps1, ps2)

def test_parse_diff_with_new_and_modified_binary_files(self):
"""Parse git diff file with newly added and modified binaries files."""
utf8_file = os.path.join(self.samples_dir, 'samples/sample8.diff')
Expand Down
8 changes: 5 additions & 3 deletions unidiff/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@

PY2 = sys.version_info[0] == 2
if PY2:
import io
from StringIO import StringIO
open_file = codecs.open
# open_file = codecs.open
open_file = io.open
make_str = lambda x: x.encode(DEFAULT_ENCODING)

def implements_to_string(cls):
Expand Down Expand Up @@ -557,10 +559,10 @@ def _parse(self, diff, encoding, metadata_only):
patch_info.append(line)

@classmethod
def from_filename(cls, filename, encoding=DEFAULT_ENCODING, errors=None):
def from_filename(cls, filename, encoding=DEFAULT_ENCODING, errors=None, newline=None):
# type: (str, str, Optional[str]) -> PatchSet
"""Return a PatchSet instance given a diff filename."""
with open_file(filename, 'r', encoding=encoding, errors=errors) as f:
with open_file(filename, 'r', encoding=encoding, errors=errors, newline=newline) as f:
instance = cls(f)
return instance

Expand Down