validate_file() and strip

Jan Lehnardt

22 years ago

Hi,
On 19 Nov 2003, at 17:24, Ilia Alshanetsky wrote:

> Any comments?

I find that useful for something I am writing right now and I was about
to
propose that myself :). I'd go for php_check_syntax() or at least
file_validate() and php_strip_whitespace() or at least
file_strip_whitespace(), but that's not too important here.

+1 if anyone cares,
Jan

--
Q: Thank Jan? -  A: http://geschenke.an.dasmoped.net/    |
Editor-in-Chief |
"We cats are very independent. We need nobody,  no time, |  PHP
Magazine   |
no where, no way. Isn't that right Pooky?" - Garfield
Key Fingerprint 7BCC EB86 8313 DDA9 25DF 1805 ECA5 BCB7 BB96 56B0

Andi Gutmans

22 years ago

I think it's a very bad idea to start copying lexer code into basic_functions.c
I don't have time to look into it but there must be a cleaner way of doing it.

Andi

At 11:24 AM 11/19/2003 -0500, Ilia Alshanetsky wrote:

Andi Gutmans

22 years ago

At 05:30 PM 11/19/2003 +0100, Jan Lehnardt wrote:

>Hi,
>On 19 Nov 2003, at 17:24, Ilia Alshanetsky wrote:
>>Any comments?
>
>I find that useful for something I am writing right now and I was about to
>propose that myself :). I'd go for php_check_syntax() or at least
>file_validate() and php_strip_whitespace() or at least
>file_strip_whitespace(), but that's not too important here.
>
>+1 if anyone cares,

If we're talking about naming the how about script_*?

Andi

Ilia A.

22 years ago

On November 19, 2003 01:33 pm, Andi Gutmans wrote:

> If we're talking about naming the how about script_*?

script_* is good, I have no naming preference. I am more interested to know
whether this functionality is something we need or not...

Ilia

Hartmut Holzgraefe

22 years ago

Ilia Alshanetsky wrote:

> script_* is good, I have no naming preference. I am more interested to know
> whether this functionality is something we need or not...

it is not needed, you can just do

system("php -l filename");

instead ;)

actually i would love to have php_check_syntax() as
it would simplify my CVS commit check scripts but i
would like to have it return the actual error message
as string on errors

IMHO it should be php_check_syntax() and not
script_check_syntax() to make clear what it actualy
checks (it won't check the syntax of my shell, awk
or perl scripts, would it? ;)

--
Hartmut Holzgraefe  <hartmut@php.net>

Ilia A.

22 years ago

On November 19, 2003 02:11 pm, Hartmut Holzgraefe wrote:

> Ilia Alshanetsky wrote:
> > script_* is good, I have no naming preference. I am more interested to
> > know whether this functionality is something we need or not...
>
> it is not needed, you can just do
>
>    system("php -l filename");
>
> instead ;)

Sure, but that's slow and more importantly, not everyone has access to php-cgi
/ php-cli to perform this action. Heck, many people do cannot use
exec,shell_exec,system,etc... functions because their providers disallow
execution funcitons.

> IMHO it should be php_check_syntax() and not
> script_check_syntax() to make clear what it actualy
> checks (it won't check the syntax of my shell, awk
> or perl scripts, would it? ;)

Good idea, we already have some function that utilize php_ prefix, might as
well continue with the trend. How about php_lint() & php_strip()?

Jon Parise

22 years ago

On Wed, Nov 19, 2003 at 02:23:44PM -0500, Ilia Alshanetsky wrote:

> Good idea, we already have some function that utilize php_ prefix, might as
> well continue with the trend. How about php_lint() & php_strip()?

php_lint() sounds goodr, and I'm cool with either php_strip() or
php_strip_whitespace().

I can see these functions being useful in various (albeit rare)
situations.

--
Jon Parise (jon@php.net) :: The PHP Project (http://www.php.net/)

Andi Gutmans

22 years ago

At 02:23 PM 11/19/2003 -0500, Ilia Alshanetsky wrote:

> > IMHO it should be php_check_syntax() and not
> > script_check_syntax() to make clear what it actualy
> > checks (it won't check the syntax of my shell, awk
> > or perl scripts, would it? ;)
>
>Good idea, we already have some function that utilize php_ prefix, might as
>well continue with the trend. How about php_lint() & php_strip()?

I hate the name lint and strip is kind of too short.
Why not keep it long like Hartmut suggested such as php_check_syntax(),
php_strip_whitespace()?

Andi

Ilia A.

22 years ago

On November 19, 2003 02:44 pm, Andi Gutmans wrote:

> I hate the name lint and strip is kind of too short.
> Why not keep it long like Hartmut suggested such as php_check_syntax(),
> php_strip_whitespace()?

Those names are fine too (personally, I dislike overly long function names,
they are harder to remember and easier to make a typo while writing the
name), although php_strip_whitespace() is somewhat misleading because the
function strips more then just whitespace.

Ilia

Hans Zaunere

22 years ago

Andi Gutmans wrote:

> At 02:23 PM 11/19/2003 -0500, Ilia Alshanetsky wrote:
>
>> > IMHO it should be php_check_syntax() and not
>> > script_check_syntax() to make clear what it actualy
>> > checks (it won't check the syntax of my shell, awk
>> > or perl scripts, would it? ;)
>>
>> Good idea, we already have some function that utilize php_ prefix,
>> might as
>> well continue with the trend. How about php_lint() & php_strip()?
>
>
> I hate the name lint and strip is kind of too short.
> Why not keep it long like Hartmut suggested such as php_check_syntax(),
> php_strip_whitespace()?

Just for consistency, the -l help string says "Syntax check only (lint)" so php_syntax_check() might be less confusing.  Also, -w strips more than whitespace...

As I heavy user of both -l and -w I'm looking forward to seeing these functions builtin.  That said, I'd prefer php_lint() since I know exactly what 'lint' means.  Of course, it doesn't make much difference at this point since I already know what the function will do :)

Hans

Jani Taskinen

22 years ago

On Wed, 19 Nov 2003, Andi Gutmans wrote:

>At 02:23 PM 11/19/2003 -0500, Ilia Alshanetsky wrote:
>> > IMHO it should be php_check_syntax() and not
>> > script_check_syntax() to make clear what it actualy
>> > checks (it won't check the syntax of my shell, awk
>> > or perl scripts, would it? ;)
>>
>>Good idea, we already have some function that utilize php_ prefix, might as
>>well continue with the trend. How about php_lint() & php_strip()?
>
>I hate the name lint and strip is kind of too short.
>Why not keep it long like Hartmut suggested such as php_check_syntax(),
>php_strip_whitespace()?

It strips more than whitespace too.. :)
And I'd favor php_strip() and php_lint(). Consistent with the
command line options too.

--Jani

Andi Gutmans

22 years ago

At 08:11 PM 11/19/2003 +0100, Hartmut Holzgraefe wrote:

>Ilia Alshanetsky wrote:
>>script_* is good, I have no naming preference. I am more interested to
>>know whether this functionality is something we need or not...
>
>it is not needed, you can just do
>
>   system("php -l filename");
>
>instead ;)
>
>actually i would love to have php_check_syntax() as
>it would simplify my CVS commit check scripts but i
>would like to have it return the actual error message
>as string on errors
>
>IMHO it should be php_check_syntax() and not
>script_check_syntax() to make clear what it actualy
>checks (it won't check the syntax of my shell, awk
>or perl scripts, would it? ;)

Yeah, I guess that makes sense :)

Andi

Ilia A.

22 years ago

Here is the revised patch. It is now much cleaner (no more lexer stuff) and
supports few additional features. By popular demand the functions have been
renamed to php_check_syntax() & php_strip_whitespace().

php_check_syntax() can now handle remote scripts and if a 2nd (optional)
argument is passe,d in the event of the parse error it'll contain the error
message, file name & line number.

php_strip_whitespace() can now handle remote scripts the rest of the
functionality remains the same.

Ilia

Andi Gutmans

22 years ago

Looks much better now. Please make sure you test it well because it's quite
sensitive APIs you are using.
About the return value, I think it should return the error message or true
on success. Then people could use === to check what they got (I prefer that
over Markus' idea). Then again I'm not completely sure about it so it
depends on what you think is the easiest.

Andi

P.S.-If you think php_strip_whitespace() is misleading then see if you can
find a better name. I think php_strip() is just too short and will be
confused with strip() (which doesn't exist but still...)

At 08:23 PM 11/19/2003 -0500, Ilia Alshanetsky wrote:

Ilia A.

22 years ago

On November 20, 2003 02:22 am, Andi Gutmans wrote:>

> Andi
>
> P.S.-If you think php_strip_whitespace() is misleading then see if you can
> find a better name. I think php_strip() is just too short and will be
> confused with strip() (which doesn't exist but still...)
> Looks much better now. Please make sure you test it well because it's quite
> sensitive APIs you are using.

Indeed, so far they pass all tests without crashes or leaks and valgrind gives
it an all clear, so I am pretty happy with it's operation at this point.
That said, if all the people who gave the functions a +1 would be willing to
try them just to see if they can make them break (hopefully fail ;) ) that
would be great.

> About the return value, I think it should return the error message or true
> on success. Then people could use === to check what they got (I prefer that
> over Markus' idea). Then again I'm not completely sure about it so it
> depends on what you think is the easiest.

Originally I thought the same thing, but comments by  Markus Fischer (in this
thread) have convinced me that it would be clearer from user perspective to
return the error by reference. We still get quite a few bug reports about
functions that should be checked with === true/false, so I am a little weary
of adding another such function.

Ilia

Markus Fischer

22 years ago

On Wed, Nov 19, 2003 at 11:24:56AM -0500, Ilia Alshanetsky wrote :

> Attached file, implements validate_file(), which implements a php space syntax
> check for PHP scripts. It returns a boolean true/false value indicating
> whether the script has parse errors or not. Essentially a "php -l" from
> within PHP.

As someone else already suggested, providing the error message would
be a good (=foreseeing) idea.

Maybe it's possible to have something like:

bool php_check_syntax($filename [, &$error_message]);

which returns the error message as the second parameter.

As a sidenote, would streams be supported (http, ftp) ? Or are there
some kind of limitating factors?

- Markus

Ilia A.

22 years ago

On November 19, 2003 02:40 pm, Markus Fischer wrote:

> 	bool php_check_syntax($filename [, &$error_message]);

I'll need to look at the code, it should be possible to do. Although I'd
prefer to have it return the error rather then return it by reference as an
optional argument.

> 	As a sidenote, would streams be supported (http, ftp) ? Or are there
> 	some kind of limitating factors?

That should be possible as well.

Ilia

Wez Furlong

22 years ago

It should work out of the box.

--Wez.

Markus Fischer

22 years ago

On Wed, Nov 19, 2003 at 03:18:34PM -0500, Ilia Alshanetsky wrote :

> On November 19, 2003 02:40 pm, Markus Fischer wrote:
> > 	bool php_check_syntax($filename [, &$error_message]);
>
> I'll need to look at the code, it should be possible to do. Although I'd
> prefer to have it return the error rather then return it by reference as an
> optional argument.

I understand. I might think it's just safer so one can assume the
return value (bool) just gives information whether it works or not
and the error message itself is a bonus.

if (!php_check_syntax("myscript.php", &$error_message) {
die ( "myscript.php didn't validaet: $error_message");
}

vs.

if (strlen($error_message = php_check_syntax("myscript.php")) > 0) {
die ( "myscript.php didn't validaet: $error_message");
}

I find the former a somewhat cleaner approach than the latter; or
maybe it's just me.

- Markus

validate_file() and strip_file()

Ilia A.

Jan Lehnardt

Andi Gutmans

Andi Gutmans

Ilia A.

Hartmut Holzgraefe

Ilia A.

Jon Parise

Andi Gutmans

Ilia A.

Hans Zaunere

Jani Taskinen

Andi Gutmans

Ilia A.

Andi Gutmans

Ilia A.

Markus Fischer

Ilia A.

Wez Furlong

Markus Fischer