Open d4mation opened 2 years ago
There are not enough test cases in your PR. Can you write some?
What tests specifically would you like to see? I'd be happy to write some, but without the refactoring necessary to be able to handle the Memo Field length, the Table Flag, and the issue I discovered in #117 I don't know how best to confirm that the file generation is occurring properly.
Changes were made specifically for Visual FoxPro (
TableType::VISUAL_FOXPRO
), but it may work for other Versions too.There are some minor issues with this PR, but they would likely require some refactoring of the library to handle nicely.
I've described the lingering issues below
Firstly, Visual FoxPro only expects 4 bytes for Memo fields, but
XBase\Header\Column\Validator\DBase\MemoValidator
will force this to 10 bytes no matter how you define the Column.https://github.com/luads/php-xbase/blob/e818e3526102a20b7d9c7e1b14feef686c973a5d/src/Header/Column/Validator/DBase/MemoValidator.php#L9-L24
As a result, the Record Byte Length for any Tables that include Memo Fields will end up being incorrect. This will also impact the offset of the Column within the Record for any Fields that come after a Memo.
Given the way that the Memo field validation is being handled "globally" I'm not sure if there is a particularly good way to handle a different byte length only for specific DBase versions.
Secondly, while the
.FPT
file is written, technically the.DBF
file has no knowledge that it exists. The Table Flag will need to be set accordingly in the.DBF
file's header if Memo fields exist.I've corrected these two issues on my end by tweaking the
.DBF
file after initial creation with the following code:Lastly, one thing that may need to be tweaked is that the Memo file is created based on whether the current Table version supports Memos rather than if any Memo Columns exist or not. Currently, every time the Table is saved (meaning every time a Record is added) it will attempt to create a Memo file even if one isn't needed. I'm not sure if there's a good way around this though, since I know the parser can be set to only load certain Columns. If the Columns were restricted like this, it may not see the Memo columns in order to make this decision.