search

whikloj / BagItTools

A PHP BagIt library
MIT License
11 stars 5 forks source link

Split BagIt spec files to allow for CR, LF or CRLF line endings #37

Closed whikloj closed 2 years ago

whikloj commented 2 years ago

Resolves #36

codecov[bot] commented 2 years ago

Codecov Report

Merging #37 (26a7e1b) into main (683b5bd) will increase coverage by 3.37%. The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main      #37      +/-   ##
============================================
+ Coverage     83.66%   87.04%   +3.37%     
  Complexity      484      484              
============================================
  Files             7        7              
  Lines          1298     1297       -1     
============================================
+ Hits           1086     1129      +43     
+ Misses          212      168      -44
Impacted Files Coverage Δ
src/AbstractManifest.php 86.66% <100.00%> (+5.78%) :arrow_up:
src/Bag.php 84.57% <100.00%> (+3.27%) :arrow_up:
src/BagUtils.php 93.00% <100.00%> (+1.16%) :arrow_up:
src/Fetch.php 91.77% <100.00%> (+4.32%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 683b5bd...26a7e1b. Read the comment docs.

whikloj commented 2 years ago

@henning-gerhardt wanted to bring this one to your attention.

This library was not following the specification in that if you had made a file with only carriage returns at the end of lines it would not validate the bag. This changes the behaviour to allow for files with any of the allowed 3 endings.

However it changes the behaviour when reading bag-info files that have random new lines for formatting and not just wrapping. This will remove the existing newline character and replace it in the libraries' bag-info copy with the OS specific character (specified by PHP_EOL).

It will not affect the bag on disk (unless you update the bag, which is the same as before). However if you were wanting to know whether the lines ended with CR, LF or CRLF it would be impossible to tell now.

I don't think this would be an issue, but wanted to ping you as you had an interest in maintaining newline characters.

henning-gerhardt commented 2 years ago

Thank you for mention me. I think this change is okay, as our usage scenario is only in creating new bags and in most cased not reading them. If we have some issues with this then I will create a new issue for this.