Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:36643
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Message-ID: <24.59.25926.4B2ECE74@pb1.pair.com>
To: internals@lists.php.net
Date: Fri, 28 Mar 2008 08:21:01 -0500
User-Agent: Thunderbird 2.0.0.12 (Windows/20080213)
MIME-Version: 1.0
References: <DA010320-E879-42BB-87BE-243CA34CBFE0@benjaminschulz.com> <47EC1271.9020405@php.net>
In-Reply-To: <47EC1271.9020405@php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: phar API
From: auroraeosrose@gmail.com (Elizabeth M Smith)

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