Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add return for deleteFromGlob #92

Open
AkramiPro opened this issue Sep 20, 2022 · 5 comments
Open

add return for deleteFromGlob #92

AkramiPro opened this issue Sep 20, 2022 · 5 comments

Comments

@AkramiPro
Copy link

AkramiPro commented Sep 20, 2022

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
Copy link
Contributor

odan commented Sep 20, 2022

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
Copy link
Author

AkramiPro commented Sep 20, 2022

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
Copy link
Contributor

odan commented Sep 21, 2022

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
Copy link
Author

AkramiPro commented Sep 21, 2022

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
Copy link
Contributor

odan commented Sep 22, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants