Conversation
|
+1. In mkopinsky@f44df3a I added a unit test/fixture for a oneline OFX file which does include some line breaks, and this PR passed that with no problem. (I also tested with the actual QFX file from Ally Bank, which also passed.) |
|
I'm glad to hear that, @mkopinsky, I will include your OFX in this PR. Thank you for your feedback. |
lib/OfxParser/Parser.php
Outdated
|
|
||
| /** | ||
| * Detect if the OFX file is on one line. If it is, add newlines automatically. | ||
| * Prepare OFX file contents. |
There was a problem hiding this comment.
Can you put a better description here of what's going on to make it clearer please? Something like:
Normalise newlines by removing and adding newlines only before opening tags
|
|
||
| return $ofxContent; | ||
| // clear all new line characters first | ||
| $ofxContent = str_replace(["\r", "\n"], '', $ofxContent); |
There was a problem hiding this comment.
Hmm, seems unlikely, but possible that there might be other newlines we didn't intend to replace here (for example, maybe a transaction description has a newline that should be kept for whatever reason). We don't have an existing test, but this may be a BC break.
lib/OfxParser/Parser.php
Outdated
| $tag = ltrim(substr($line, 1, strpos($line, '>') - 1), '/'); | ||
|
|
||
| // Line is "<SOMETHING>" or "</SOMETHING>" | ||
| if ($line == '<' . $tag . '>' || $line == '</' . $tag . '>') { |
There was a problem hiding this comment.
Please use === for equality checks
asgrim
added
the
enhancement
|
Nice improvement, thank you @berosoboy - just a couple of changes please as above :) |
|
Sorry for the delay on this one, there seems to be a conflict - could you take a look please and poke me when resolved? Thanks! |
3 participants
Fix #34