Ne-Lexa / php-zip

PhpZip is a php-library for extended work with ZIP-archives.
MIT License
491 stars 60 forks source link

add return for deleteFromGlob #92

Open AkramiPro opened 1 year ago

AkramiPro commented 1 year ago

hi please add some return or throw for methods like deleteFromGlob for when path is not valid and no file exist to delete there is no way to find out the result of this methods please fix that

odan commented 1 year ago

There is a way to test it. Of course, an internal Exception would be better, I know.

$zipFile->deleteFromGlob('**.{xml,json}');

if(isset($zipFile['composer.json'])); {
    throw new \PhpZip\Exception\ZipException('The file could not be deleted');
}
AkramiPro commented 1 year ago

thank you for your reply but there is another scenario that the path is not exit at all . like when i use unlike php method and if i pass invalid path then i got warning in my case i write dynamic builder that create zip with pre config file and it is very important to get error if any config option is not true like delete option for ex i need to get this error : the path is not valid or of cures may be it is not necessary in all scenario and you can add an option in global class for strict mode like php https://dev.to/rocksheep/the-way-stricttypes-works-in-php-eb7

odan commented 1 year ago

I guess you are mixing some concerns. This has nothing to do with the PHP language and the script mode. To make sure that your application specific config options are valid and that the source path exists, you may use a validator for that specific use case.

AkramiPro commented 1 year ago

i just give you an example that how php handle such errors . i mean you should do this as well and add some option like php so developers can see all errors in your code for now many actions in your lib just do the job without any result and log! for ex i see the source code and in some case you do actions for each item in zip file in loop so if one item has error we can't get it ! for better understanding give you this ex: i can't trust your methods when they return true because most of them are in loop and don't check result for each item and just after loop return true ! like deleteFromGlob that use loop

i hope you get what i mean thank you for your reply

odan commented 1 year ago

Just to be clear. My answer was only related to your first question. I know how errors should be better handled. Note that I'm not a maintainer of this package and that this is not "my" code. Also note that the deleteFromGlob is, as the name implies, a method that uses a glob pattern to delete files from a ZIP file. A glob pattern describes what to delete, if it exists. If it does not exist, it will not be deleted. So from this perspective it's even not an "error" if a file, which does not exist, cannot be deleted. This is what a glob is for, by design. This means you may need another approach to check if your path or settings are correct or not. I hope it helps.