First of all Greg, thanks for all your hard work on phar - however I do
agree that some of the API choices are going to cause confusion with
"casual" phar users.
> 1) if you want to know writability without Phar::canWrite(), you can
> also use is_writeable('phar:///path/to/archive.phar/index.php') just
> like you can with any other file
However Phar::canWrite(); merely tells me if the readonly ini choice is
flipped, correct? It doesn't relate to the actual file. While the
is_writeable function will do what I want for a specific file - I agree
that a $phar->isWriteable() method would be a nicer (and more intuitive
for the OO nuts) option.
> 1) file_put_contents('phar:///path/to/archive.phar/index.php', 'file
> contents');
> 2) $phar = new Phar('/path/to/archive.phar');
> $phar->buildFromIterator(new DirectoryIterator('/some/path'),
> '/some/path');
> 3) $phar = new Phar('/path/to/archive.phar'); $phar['index.php'] =
> file_get_contents('/some/path/index.php');
I don't know about you, but the common way I work with archives is to
build a directory structure and just shove it into an archive wholesale
- although the second option allows this - it's very verbose! Any
chance of a $phar = new Phar('/path/to/phar', '/path/to/archive');
option for the lazy? Or even a shortcut static
Phar::create('/path/to/phar', '/path/to/archive') - and no I don't want
to have to do an iterator too unless I'm worried about filtering what's
going in, extra steps are usually not a good thing. the option to have
it is good, the choice to force it is not.
> There's plenty of flexibility here, and no need to be scared of the
> arrayaccess option.
It's not so much a question of being scared as a question of retraining
- being forced to learn a "new way of doing things" for one extension is
frankly annoying.
> I do agree that although I am perfectly comfortable with the current
> API, it may seem inconsistent to others who think the way you do to
> directly set contents that way when offsetGet() does not return the
> contents. However, the solution is quite simple, which is to make even
> clearer what operation we are doing:
>
> $contents = $phar['index.php']->contents;
> $phar['index.php']->contents = 'something';
>
> This way, it is even clearer that the archive is an "array" of
> PharFileInfo objects. This would be very easy to implement and would
> not require removing the existing functionality.
>
> I see no problem in adding an alias to offsetSet() called addFile() and
> another for addFromString() and a method for addEmptyDir(). In case it
> isn't obvious, I will not do this change unless it adheres to the
> existing ext/zip API to allow easy mental migration for developers
> between the two extensions, so don't even think about suggesting other
> possibilities, thank you :).
That sounds like a great idea - the two extensions SHOULD have
compatible API's - however they shouldn't deviate from the basic mental
concept of how files and directories work either. In other words
->addFile(file) and ->addDirectory(do_recursive) and ->addEmptyDir()
would keep people from head scratching. And keep us from having to
create an iterator just to shove a directory into a phar.
Thanks for your hard work Greg - the functionality looks great, I'd just
like a bit of consistency with other extensions and commonly used PHP
archive manipulation classes so retraining isn't in order just to use phars.
Looking forward to more fun!
Elizabeth
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
