jatpat / phpliteadmin

Automatically exported from code.google.com/p/phpliteadmin
0 stars 0 forks source link

Split Source into multiple files during development and merge with Build script #190

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Currently we develop PLA in one file. The one-file approach is good for the 
release (install, deployment...), but bad for development. It can lead to lots 
of unnecessary merging. And by splitting the code into several files, it would 
be easier to find the relevant parts in the code etc.

So I would propose something like this for a start:

- phpliteadmin.php: Everything currently in there, except:
- defaulttheme.css: Default JS theme
- script.js: JavaScript. Possibly splitted into multiple files.
- favicon.ico: The favicon
- One file per (PHP) Class

This should be good start. the goal should be to move more and more code out of 
the phpliteadmin.php into Classes.

Then we need a build-script that automatically bundles everything into one 
phplteadmin.php file again that we can release.
This build script would more or less only paste the individual file data inside 
the phpliteadmin.php. Maybe do some compression of JS and CSS.

During development, it is important that PLA runs without the need to run the 
build script every time.
So basically, I think in phpliteadmin.php, we move some stuff out and replace 
it with include()s. The build-script then only replaces the include()s by the 
actual file content.

Original issue reported on code.google.com by crazy4ch...@gmail.com on 9 Mar 2013 at 12:38

GoogleCodeExporter commented 9 years ago
(this was mentioned as a comment in issue #157).

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 12:39

GoogleCodeExporter commented 9 years ago
if i do it, i would do something like this (on image). templates would be in 
ob_start or somewhere at the end of file.

So, everything will be in classes, while start up will look like

Factory::getApp()->init(debug)->dispatch();
or
Factory::getApp()->init(debug)->route()->dispatch();

Original comment by ykoro...@gmail.com on 9 Mar 2013 at 6:17

Attachments:

GoogleCodeExporter commented 9 years ago
This issue is not about improving the architecture of PLA, it is about 
splitting files to simplify development. Or to rephrase it: It is not about 
restructuring or changing any code. It is about splitting the code that is 
already existing. Improving the development infrastructure, not PLA itself.

Restructuring, like introducing an MVC architecture, as implied by your draft, 
is another issue. Although this could largely benefit from this issue (being 
infrastructure that supports restructuring).

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 6:25

GoogleCodeExporter commented 9 years ago
SQLite itself also is developed in lots of files (of course ;-)) - see 
https://www.sqlite.org/cgi/src/dir?ci=e899b058a7031580&name=src ) and then the 
make script builds one big file out of it. See 
https://www.sqlite.org/amalgamation.html .

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 6:40

GoogleCodeExporter commented 9 years ago
I see. But you already do it with version 2, right? Or version 2 is dead?

for better merging you have to remove duplicates of code and remove global 
variables, otherwise, it will be hard to get what variable is set and where.

it is not mvc, your structure is simple, so I suggest to extend your app with 
configuration and registry classes. factory is also good.

also many methods have many if-else, it is very hard to read (for me), but if 
it is good for you, nevermind then. Still I'm somewhere on line 900.. so don't 
get me too serious

Original comment by ykoro...@gmail.com on 9 Mar 2013 at 6:42

GoogleCodeExporter commented 9 years ago
about merge script, it can be like this

MERGE.bat
copy test1.txt+test2.txt test3.txt

Original comment by ykoro...@gmail.com on 9 Mar 2013 at 6:48

GoogleCodeExporter commented 9 years ago
>  Still I'm somewhere on line 900.. so don't get me too serious
lol. I have never read the code like this.

> I see. But you already do it with version 2, right? Or version 2 is dead?
This was planned for version 2, but nobody currently develops version 2, so I 
think it won't every come to live. Instead we incrementally improve the current 
version until we have what was planned for version 2. One of these steps is to 
split files. For version 2, the build-process was not implemented yet, so we 
cannot use anything from there.

> for better merging you have to remove duplicates of code and remove global
> variables, otherwise, it will be hard to get what variable is set and where.
I am not sure what you mean. I plan to only move classes out of the 
phpliteadmin-file. So there should be almost no global stuff used in there.

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 6:51

GoogleCodeExporter commented 9 years ago
> MERGE.bat
> copy test1.txt+test2.txt test3.txt

No, that is not what I have in mind. The problem with this would be that you 
could not run PLA without running merge.bat after every change. This would make 
development harder (more annoying / requiring better IDEs).

Instead, look at this example:
phpliteadmin.php:
<?php

...

#INCLUDE
include('classes/Authorisation.class.php');

..
?>

So we can run phpliteadmin.php during development without the need to run the 
build-script.

Then we have a buildscript that replaces the include() marked by the #INCLUDE 
marker with the file referred to.
I think for portability, we should best implement this in php. A windows batch 
file or linux make file does not run on both platforms, so we would need to 
write and maintain 2 of them.

I thought of something like:
build.php
<?php

$source=file_get_contents('phpliteadmin.php');
preg_match_all('/#INCLUDE\n\s*include\(\'([^\']+)\'\);/',$source,$matches);
foreach($matches as $match) {
 $include_source = file_get_contents($match[1]);
 $source = str_replace($match,$include_source,$source);
}
file_put_contents('built/phpliteadmin.php');
?>

(just a draft to illustrate what I mean, not tested etc)

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 7:06

GoogleCodeExporter commented 9 years ago
bat is enough to make bulk file, you can add/update bulk file in svn.
For developer it is enough to have entry point like index.php which won't be 
included by bat file.
about linux, it is simple as is
$cat test1.txt test2.txt >> test3.txt

i guess, you will have to have merged file in svn too. or you don't want to 
keep this file in repository?

you can merge with php, the main question is - who will merge files?

Original comment by ykoro...@gmail.com on 9 Mar 2013 at 7:37

GoogleCodeExporter commented 9 years ago
Hmm. Well, yes, we could have an index.php that includes all other files to run 
PLA during development like this:

index.php
<?php
include('file1.php');
include('file2.css');
include('file3.js');
include('file4.php');
...
?>

We would need to make sure links work (i.e. point to the correct php-file) 
then, but this would be easy. And we could have a batch script for linux and 
windows to concat these files like your examples.
But:
We specify the list of files to include in 3 places then: the index.php, the 
windows build script and the linux build script. This can get inconsistent 
easily.

That's comparably easy to fix, all 3 scripts could simply use all files form a 
certain directory automatically. But then, in which order? We would need to 
enumerate files. And then, what if we put a new file in between? rename all 
following files?

Another way would be to have another central config-file for all 3 (build) 
scripts (like filelist.txt) that lists the files in the order to include them.

But I think the solution of having normal php includes that get replaced by a 
build script still is a lot nicer and easier to figure out.

> i guess, you will have to have merged file in svn too. or you don't want to 
keep
> this file in repository?
Good question. I guess we should not include it in the repository. The merged 
file is for the release. It would change with every commit or would be 
inconsistent with the other files.
I think the best approach would be to setup a post commit web hook (which is 
supported by google code) that automatically runs the build script after every 
commit and makes the built file available on some webserver.

This would allow us to always have a 1-file development version of PLA 
available. for example, I like to post links of the current development version 
to users so they can check if the bugfix works for them. It would not be so 
easy for them to checkout the whole source with multiple files.

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 8:36

GoogleCodeExporter commented 9 years ago
By the way: As I mentioned, minifying files might be an additional task that 
could be performed by the build script. So compressing js/css files, maybe even 
the php-file itself.
Would be better to have a central automatically running build script that does 
this than to have linux and windows batch scripts that do this.
Minify is a cool php script for this for example ( 
https://code.google.com/p/minify/ ),

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 8:50

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
what about next aproach: index.php?merge

index.php
<?php
$files = array('file1.php', 'file2.php',...);

if(isset('merge')) {
$buf ='';
foreach($files as $f) { $buf .= file_get_content($f); }
file_put_content('pla.php', $buf);
echo 'merged successfully';
die;
}

foreach ($files as $f) { include  $f; }

Original comment by ykoro...@gmail.com on 9 Mar 2013 at 9:58

GoogleCodeExporter commented 9 years ago
Hmm. That solves the problem of portability (by using php) and consistency (by 
using a single file-list).

I think that approach is okay.
But I still see a disadvantage. Example:

file1.php
<?php
//part1
//part2
//part3
?>
Now you want to move part2 into a separate file.
Then you need to create 3 files, ripping apart part1 and part3, although they 
might belong together:

file11.php <?php // part1 ?>
file12.php <?php // part2 ?>
file13.php <?php // part3 ?>

With my approach, you could insert an include between part1 and part3 so they 
stay together.

Any other opinions?

Original comment by crazy4ch...@gmail.com on 9 Mar 2013 at 10:35

GoogleCodeExporter commented 9 years ago
So much to read suddenly :)  A couple of sparse comments before I get to the 
point:
- we have php, let's avoid shell or batch scripting
- rather than include, this is a job for 'require'
- include/require are not functions, no need for brackets

That said, I wouldn't use either of them (kinda). It's 2013 and we've had class 
autoloading for some time, spl_autoload¹ starting from php 5.1.2, and PSR² 
stuff is looming.

I'd like to split classes into separate files and autoload them (and yes, that 
requires 'require's) and for release we simply concatenate the main file with 
the class files, without changing a line of code. The build script can be php 
and handle resource inclusion, minifying and encoding —so we can use 
php-minify instead of YUI Compressor.

The split version and bundled version should live in parallel, to make 
switching from one to the other (that is, development) easier. I'd suggest this 
directory structure

/-- (version directory)
  |
  +-- build-pla.php
  +-- index.php ( main pla script, non-oo code )
  +-- phpliteadmin.config.php ( personal, works for both versions, not in repo )
  +-- phpliteadmin.config.sample.php
  +-- phpliteadmin.php ( built by script )
  +-- resources/*  ( css/js/etc )
  `-- classes/*    ( ClassName.php )

The easiest way to embedding resources would be to have some placeholders 
inside the code, but maybe we can work around that with a smarter Resources 
class.

[1] http://www.php.net/spl_autoload
[2] http://www.php-fig.org/

Original comment by dreadnaut on 10 Mar 2013 at 11:21

GoogleCodeExporter commented 9 years ago
I had a look at php minify, the script linked above, and I'm a bit uncertain. 
Of the whole package, we need only two classes (Minify_JS_ClosureCompiler and 
Minify_CSS_Compressor), but the first contacts an online service to compile 
javascript and will not work without an internet connection.

I'll look around, hopefully there's a similar php solution with a 
self-contained implementation.

Original comment by dreadnaut on 13 Mar 2013 at 4:00

GoogleCodeExporter commented 9 years ago

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 12:49

GoogleCodeExporter commented 9 years ago
I was just trying the split + autoload system now and a 5-lines function is 
enough to hold everything together.

If you like the method I can prepare a patch and commit it first thing after we 
release 1.9.4, so we can then work on the build script in parallel with other 
development.

Original comment by dreadnaut on 14 Mar 2013 at 12:52

GoogleCodeExporter commented 9 years ago
Yes, autoload definitely seems to be the correct way to load classes, better 
than the other 2 approaches.
I think this should be the first step, so yes, please prepare a patch.

We then need to make sure it is easy to edit css & js while developing (for css 
it is already easy as we can set $themes to the css file and that's it. But for 
JS we need to do something similar.)

> I had a look at php minify, the script linked above, and I'm a bit uncertain.
Well. In my opinion, we could simply setup the build script on some webserver 
so it always automatically builds the current development version. Then we 
could always have a built 1-file-version available. Nobody would need to setup 
the build script anywhere else.
Then it would be no problem if we only need a small portion of minify or if it 
requires an internet connection. But this also means we could use YUI 
compressor for example.
I am not sure if minify uses this as well, but Google also offers a JS minify 
API: https://developers.google.com/closure/compiler/

But if you find some great php class that does our job so we can create a small 
php build script that anybody can setup easily, of course this would be a lot 
better.

When we are done with this issue, we should create a wiki page that explains 
some things on how to develop PLA most easily.

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 11:51

GoogleCodeExporter commented 9 years ago
Ah. On another project, I was using these 2:
http://code.google.com/p/cssmin/
https://github.com/rgrove/jsmin-php/

The later one is unmaintained and says "You shouldn't use it." ;-)
(Nevertheless it worked great when I used it...)

It recommends this one (and google closure I mentioned above):
https://github.com/mishoo/UglifyJS2
But this is not php, it is js (node.js).

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 11:57

GoogleCodeExporter commented 9 years ago
Here are some more to check out:
https://github.com/tedivm/JShrink (looks good at first sight)
http://crisp.tweakblogs.net/blog/cat/716

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 12:05

GoogleCodeExporter commented 9 years ago
There a JSMin+ but I'm not sure of its status. I'll look around, but if we 
don't mind java the YUI Compressor might be one of the best solutions.

About autoloading, this is what I tried:

@@ -361,6 +361,20 @@
 define("FORCETYPE", false); //force the extension that will be used (set to false in almost all circumstances except debugging)
 define("SYSTEMPASSWORD", $password); // Makes things easier.

+// setup class autoloading
+function pla_autoload($classname)
+{
+   $classfile = __DIR__ . '/classes/' . $classname . '.php';
+
+    if (is_readable($classfile)) {
+      include $classfile;
+      return true;
+    }
+    return false;
+}
+
+spl_autoload_register('pla_autoload');
+
 // Resource output (css and javascript files)
 // we get out of the main code as soon as possible, without inizializing the session
 if (isset($_GET['resource'])) {

Original comment by dreadnaut on 14 Mar 2013 at 12:07

GoogleCodeExporter commented 9 years ago
Yes, looks good. But didn't you say yourself that this is a job for require? ;-)

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 12:14

GoogleCodeExporter commented 9 years ago
Not in an autoloader :-p  If you can't find or load the file you should 
not halt the script, but return false and let the next autoloader in the 
chain try.

Original comment by dreadnaut on 14 Mar 2013 at 12:22

GoogleCodeExporter commented 9 years ago
Okay, true. But as long as there is only one autoloader, it doesn't really make 
a difference, right? "Class not found" is a fatal error, so PHP will halt 
anyway if a class is missing (once used, and autoloaders don't get called if 
the class is not used).

Original comment by crazy4ch...@gmail.com on 14 Mar 2013 at 12:28

GoogleCodeExporter commented 9 years ago
| But as long as there is only one autoloader, it doesn't really make a 
difference, right?

Correct, it would be the same *if* there were only one autoloader. If someone 
integrates PLA with their software there might be more than one, so we 
shouldn't break the autoloading chain for no good reason.

Attached is a sample split version so we have somewhere to start from. It uses 
Minify's CSS compressor class and JShrink for javascript (both stored inside 
the support/ directory). index.php contains the main code, while each class has 
its own source file under classes/.

The build script concatenates the main index.php with classes/*.php, removing 
the first php opening tag. Resources are then prepared and replace any 
occurrences of :::filename::: in the source code. I modified the Resources 
class accordingly to work in split mode: if the resource data contains 
:::filename::: it will load the external file.

Original comment by dreadnaut on 14 Mar 2013 at 2:54

Attachments:

GoogleCodeExporter commented 9 years ago

Original comment by dreadnaut on 18 Mar 2013 at 4:29

GoogleCodeExporter commented 9 years ago
New version, based on the code released for 1.9.4. Same changes as above: split 
classes, added autoloading (about line 390), added a basic build script.

Should we place languages in a subdirectory? The root directory might get messy 
as we get more locale files, but we might also have to change some code to 
accomodate that. I think themes now do something similar, searching first in 
the current directory and the under themes/.

Original comment by dreadnaut on 18 Mar 2013 at 6:25

Attachments:

GoogleCodeExporter commented 9 years ago
Languages also get loaded from "languages". No problem to move them there. 
Works already.

We should think about how we deal with the English stuff. Because I think it 
would be good to move this outside of the main file as well.

(I have not had a look at your upload yet, will do so in a minute)

Original comment by crazy4ch...@gmail.com on 18 Mar 2013 at 6:28

GoogleCodeExporter commented 9 years ago
Looks like a very good start. I really like the Resource handling.
It introduces 3 LOC that are useless in the build version, but that's clearly 
worth it.

I would do the replacement of resources only on the code of the Resource class. 
Otherwise we introduce assumptions like "you must not have 
:::resources/phpliteadmin.css::: anywhere in your code". Although this 
assumption is quite safe, I would prefer to restrict it to the Resource class.

About the <?php removal: I was thinking about whether it would be better to 
simply add ?> at the end of every php file. Then concatenation would work 
without removing "<?php". On the other hand, some useless "?>\n<?php" will end 
up in the code. Maybe your solution is better.

As said before, I would prefer to move the English language definition out of 
index.php as well. At the moment we have it in 2 places, which can lead to 
redundancy problems.
We could either implement something special in the Build-Script or change the 
language system somehow so it is a class that gets autoloaded to achieve that.

When I look at what remains in index.php, this looks like quite a lot of work 
until we get this "nice".

We should remove or adjust the #eof comment because with the build script it 
ends up in the middle of the file ;-)

Original comment by crazy4ch...@gmail.com on 18 Mar 2013 at 6:54

GoogleCodeExporter commented 9 years ago
Closing php tags are a risk, you might end up sending a couple of newlines to 
output. With autoloading, files are included on the fly when you try to 
instantiate a class, which means that those newlines might end up in output 
before headers. Bugs waiting to happen IMO :)   Saying "the first line of a 
class file must be <?php" is a tame requirement in the end. The same for "class 
files should produce no output on loading."

I'm uncertain about the Resource implementation, and I'll see if I can do 
better using __halt_compiler()¹. The current code mixes code and data too 
much, and would not scale well. That would also solve the :::something::: 
replacement in other parts of the code.

English strings: good idea, although we can do that later. Let's split the 
source as soon as possible, as it will make working on multiple changes easier.

| We should remove or adjust the #eof comment because with the build script it 
ends
| up in the middle of the file ;-)

Ah, yes! Maybe replace it with #main-code-ends-here, or just drop it.

[1] http://php.net/manual/en/function.halt-compiler.php

Original comment by dreadnaut on 18 Mar 2013 at 11:18

GoogleCodeExporter commented 9 years ago
I'm on it anyway, taking ownership :)

Original comment by dreadnaut on 19 Mar 2013 at 12:37

GoogleCodeExporter commented 9 years ago
| Closing php tags are a risk, you might end up sending a couple of newlines to 
output.
Ah, right. Although you can end a file without a newline (and thus concatenate 
files without introducing one), some editors tend to add newlines at the eof, 
so your solution really seems to be safer.

| I'll see if I can do better using __halt_compiler()
I did not know this one. In the end it is only an "exit;" with the advantage 
that you can easily jump to __COMPILER_HALT_OFFSET__ and don't have to 
determine the position yourself. But looks very suitable in our case.

| English strings: good idea, although we can do that later.
Okay.

| Maybe replace it with #main-code-ends-here, or just drop it.
I think a comment like this might be useful (e.g. to search for the position).

| I'm on it anyway, taking ownership :)
Thanks.

Original comment by crazy4ch...@gmail.com on 19 Mar 2013 at 5:07

GoogleCodeExporter commented 9 years ago
Ok, here's a different take on resources AND building.

As I wrote above I am switching to __halt_compiler(), not only works as 'exit', 
but because whatever follows it is *completely ignored*: not parsed, not sent 
to output, nothing. You can have megabytes of binary madness after the call, 
and they won't affect the execution.

This however makes the build script a bit more complicated, so I've added an 
external "build template" which contains directives to include the right files 
in the right position. Check 'build.php' for more details, I spent some time 
document the script so it's not black magic.

A few changes to the Resources class as well, which now calls 
getInternalResource() to load data from the script file itself. It's not the 
*cleanest* implementation, but I think better than the previous version.

Original comment by dreadnaut on 19 Mar 2013 at 7:23

Attachments:

GoogleCodeExporter commented 9 years ago
Sorry, there was a duplicated line in classes/Resources.php (line 53) which 
broke split-file execution. Attached is the correct file.

Original comment by dreadnaut on 20 Mar 2013 at 2:28

Attachments:

GoogleCodeExporter commented 9 years ago
Just had a look t your new solution "b". Looks very cool. I consider this more 
or less an advanced version of what I had initially in mind. Very flexible.

What I noticed is that in the build script, you always work on $output. This 
means in case any of the files you put in $output contains some build-command 
interpreted next, it will get replaced.
This means for example that #EMBED commands could also be placed inside of 
index.php or classes (but not #INCLDUE commands). I am still thinking about 
whether this can be useful or if it is rather dangerous.
(As resources get base64 encoded, no build-command can accidentally get 
introduced in there as they require "# " which cannot be base64. )

If we could also put #INCLUDE commands in the index.php, this would mean we 
could include lang_en.php this way. But then we could also put everything in 
index.php and wouldn't need the template...

But I mean in the end, this is also a suitable solution that we could use and 
improve over time.

Original comment by crazy4ch...@gmail.com on 20 Mar 2013 at 5:26

GoogleCodeExporter commented 9 years ago
Great feedback, thanks Chris. I am indeed following your first post :)

Regarding working on $output, that's a good point, I missed it. We can work 
around it by first processing EMBEDs first and INCLUDEs second, though. I'd 
rather not have #EMBED commands appearing in other files: code in one place, 
data in another.

A first version of the build script didn't use an external template and the 
directives were all at the bottom of index.php. It felt a bit dirty, so I 
switched to a separate file.

We could #INCLUDE the english translation as default. At that point the 
external template cannot be used though, since the lang data must appear toward 
the top of the file —unless we also separate the license text and the default 
config (oh, wait we already do the latter!)

I see two alternatives then:
a) everything in index.php
b) split more!

In (b) we end up with a template more or less like this:

<?php
#INCLUDE docs/header.txt | comment_the_whole_thing
#INCLUDE phpliteadmin.config.sample.php
#INCLUDE languages/lang_en.php
#INCLUDE index.php

// resource embedding code
function getInternalResource($res)
{
  ...
}

#EMBED resources/*
[...]

I like including the default config, although this brings back some of the code 
and comments we removed when we added the sample config.

Original comment by dreadnaut on 20 Mar 2013 at 9:17

GoogleCodeExporter commented 9 years ago
I also like b)
More splitting is fine as long as we don't introduce too much coupling of 
different files.
Splitting everything out of the main file that is "a thing of its own" is good.
And default-config and the header comments can really be considered a "thing of 
its own".
Aside: Thinking about the header: We could replace the manually updated "last 
updated" comment in the header with a date inserted by the build-script.
#INCLUDE docs/header.txt | insert_build_date | comment_the_whole_thing

> I like including the default config, although this brings back some of the 
code 
> and comments we removed when we added the sample config.
Well, the very good point of it is that we reduce redundancy. If we add another 
config variable, adding it to one file would be enough (at the moment, we'd 
need to add it in 2 files).
About the comments and code: Well, I don't mind much about this. But if we want 
to, we can also make our build script smart enough to remove some lines (marked 
somehow?).
#INCLUDE languages/lang_en.php | remove_marked_lines

Original comment by crazy4ch...@gmail.com on 20 Mar 2013 at 10:06

GoogleCodeExporter commented 9 years ago
Last line was meant to be:
#INCLUDE phpliteadmin.config.sample.php | remove_marked_lines

Original comment by crazy4ch...@gmail.com on 20 Mar 2013 at 10:08

GoogleCodeExporter commented 9 years ago
Currently, the build script remove all single-line comments starting with # 
(rather than //), even if they are not directives. I used it to add "build 
comments" here and there but is not essential to my implementation.

We can make it a convention and use it to remove lines. Otherwise, we can strip 
lines between to special comments, e.g.:

# IGNORE
function should_not_be_in_the_single_file_code()
{
  ...
}
# END IGNORE

Last updated: I'd like to automate that and add it only to release and RC 
versions. I always forget to update it, and it will be even more difficult 
after the split, since we would have to commit at least two files every time!

Original comment by dreadnaut on 20 Mar 2013 at 10:55

GoogleCodeExporter commented 9 years ago
I've had a look at implementing comment #37 and building works alright. 
However, we need to include the default configuration otherwise for the 
split-file version to run :|

Something like this would work, but maybe there's a nicer way.

# IGNORE
include 'phpliteadmin.config.sample.php';
# END IGNORE

Also, following Issue #197 and Lonnie's email* pointing out that we cannot give 
zlib for granted, I'm going to remove gzencoding from the build script.

[*] https://groups.google.com/d/msg/phpliteadmin/gipdRoGVkjo/VIntw7P7cF0J

Original comment by dreadnaut on 21 Mar 2013 at 3:38

GoogleCodeExporter commented 9 years ago
Sorry, link to the wrong message, here's the correct one:
https://groups.google.com/d/msg/phpliteadmin/gipdRoGVkjo/H8WevWOmUqkJ

Original comment by dreadnaut on 21 Mar 2013 at 3:39

GoogleCodeExporter commented 9 years ago
> Something like this would work, but maybe there's a nicer way.
Hmm. We could do something like this, but I think it's not better than your 
solution:

if(!isset($databases))
  // can only happen in split-mode, as the built version defines it
  include 'phpliteadmin.config.sample.php';

But the #IGNORE build-command could be useful anyway I think.

> we cannot give zlib for granted, I'm going to remove gzencoding from the 
build script
Yes. Zlib is always built-in on Windows, but not on Linux where you need to 
compile php with zlib and this is not the default. See 
http://www.php.net/manual/en/zlib.installation.php
So we should not rely on it. If we use it, we first need to make sure it's 
available. But as the use is not important in this case, we probably better 
shouldn't use it at all.

Original comment by crazy4ch...@gmail.com on 21 Mar 2013 at 3:50

GoogleCodeExporter commented 9 years ago
I'll go for an "#IGNORE" syntax then. Although... do you have any suggestions 
for a better keyword?

#SPLIT-ONLY, #DON'T-BUILD, #BUILD:NO, #DEVEL —I can't find a good one :|

For reference: we can use extension_loaded('zlib') to check for support.
http://www.php.net/extension_loaded

Original comment by dreadnaut on 21 Mar 2013 at 3:59

GoogleCodeExporter commented 9 years ago
Hmm. This command will make it not only in the template, but in the index.php. 
So some guys hacking pla might see it and we should give it a name so they know 
what's going on.
My suggestion:
# REMOVE-FROM-BUILD
...
# END REMOVE-FROM-BUILD

Original comment by crazy4ch...@gmail.com on 21 Mar 2013 at 4:36

GoogleCodeExporter commented 9 years ago
Ooook, maybe we are getting there. Here's version -C-.

- added # REMOVE_FROM_BUILD ... # END REMOVE_FROM_BUILD build directive to 
ignore lines, available both in the build template and in included files;
- added ###identifier### build directive to be replace with the content of a 
variable, available in all files.

The former is used to include the default files in split-mode, and the latter 
is used to insert $build_date in header.txt. Maybe ###[\w+]### is not "safe" 
enough?

- new build template that includes docs/header.txt, the sample config file and 
the English strings;
- removed zlib compression, resources are now only minified;
- swapped EMBED and INCLUDE to avoid embedding resources in included files; 
EMBED should now work only in the build template;
- added comment_lines filter to comment header.txt.

Original comment by dreadnaut on 21 Mar 2013 at 7:15

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks!
> added # REMOVE_FROM_BUILD ... # END REMOVE_FROM_BUILD build directive to 
ignore 
> lines, available both in the build template and in included files;
Hmm. Does this make any sense in the build template? It will only be used when 
building. Having something in there that is removed while building is a bit 
weird. Well, it's some kind of multiline-comment, okay. (I know, it is easier 
to do it this way, and as it doesn't introduce any problems, it's perfectly 
fine.)

>  Maybe ###[\w+]### is not "safe" enough?
Hmm. If the build-script acted as documented in the comments, I'd say it is 
safe enough:
// ###identifier###
// Is replaced with the value of the variable $identifier, if defined

"if defined" is not what the script does. If I append ###bla### to header.txt, 
it will get removed in phpliteadmin.php (i.e. replaced by nothing). Instead the 
build script should look up if the variable is set and skip ###bla### because 
$bla is undefined.

If it acted like this, I would consider it "safe enough" because only very few 
###identifier### are valid.

To improve this, I would explicitly introduce an array of stuff that can be an 
identifier like
$identifiers['build_date']=date('Y-m-d');
This would make sure only what is clearly defined as an identifer will get 
replaced. Otherwise, this could cause serious trouble:
// This is where the ###output### of the script starts

But we don't have to make this perfect now. We can start using it and see how 
good it works and improve it over time.

Original comment by crazy4ch...@gmail.com on 21 Mar 2013 at 8:58

GoogleCodeExporter commented 9 years ago
| "if defined" is not what the script does. If I append ###bla### to header.txt,
| it will get removed in phpliteadmin.php (i.e. replaced by nothing)

Ah, I knew I had forgotten something. Fixed it: if the variable is not defined, 
the string will not be replaced. Also, I added a $build_data array (similar to 
$identifiers as you suggested) which works for both ###something### and #VAR, 
which I have renamed as #EXPORT, though.

I also reshaped the build script comments into some kind of documentation, and 
added a few checks and warning when unexpected stuff happens (missing files, 
undefined variables).

I fairly happy with this version, so if no-one has further comments on it I'm 
ready to commit it under 1.9.5.

Original comment by dreadnaut on 22 Mar 2013 at 3:02

Attachments:

GoogleCodeExporter commented 9 years ago
Thanks a lot for your great work!

Looks very good. I think we can start using it, so please commit it in 1.9.5.
But please have a look at the commits that have been made to the 1.9.5 branch 
and make sure they won't get reverted.

Original comment by crazy4ch...@gmail.com on 23 Mar 2013 at 1:05

GoogleCodeExporter commented 9 years ago
Here we go: rebased on the latest 1.9.5 and committed as r385.

Original comment by dreadnaut on 24 Mar 2013 at 4:35