SyuTingSong / 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
Of course I made a mix-up while diffing my index.php with the code from 1.9.5. 
with r386 I noticed a first sign, and r387 fixes the last issues —mainly, I 
forgot to remove some of the code split to external files.

Original comment by dreadnaut on 24 Mar 2013 at 5:23

GoogleCodeExporter commented 9 years ago
Thanks a lot!

I just had another idea: With this build-system that works much like 
conditional compilation in C (#IFDEF), we could offer different builds of PLA 
like one with support for SQLitev2 and one only for v3. I think there should be 
a bunch of code in the DB-class (and, unfortunately, not only there :( ) that 
is only a sqlite2 workaround.

But let's forget this, we have a lot more important things to do. Saving 10 KB 
of size for some users probably will never be worth the effort of releasing and 
maintaining multiple versions.

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

GoogleCodeExporter commented 9 years ago
I noticed a problem:

Currently, custom functions are defined both in phpliteadmin.config.sample.php 
and in (built) phpliteadmin.php.

Thus, renaming phpliteadmin.config.sample.php to phpliteadmin.config.php yields 
this error:
Fatal error: Cannot redeclare md5rev() ...

I see two solutions:
1. insert # REMOVE_FROM_BUILD into phpliteadmin.config.sample.php
+ This will make phpliteadmin.php a bit smaller.
- But users might be confused by the comments (but removing them does not break 
anything as long as they don't build pla).
2. function_exists() around all custom function declarations.
+ Users understand function_exists()
-- the function in phpliteadmin.php gets used because it is defined earlier. 
Users will be confused that changing phpliteadmin.config.php does have no 
effect.

So I guess we need to go for 1.

Original comment by crazy4ch...@gmail.com on 25 Mar 2013 at 11:24

GoogleCodeExporter commented 9 years ago
ah, 1 more disadvantage of 1.) :-(
Does not work the same way during development. index.php will first include the 
sample-config and then the user config. Build command has no effect here. This 
means developers must remove custom functions from their user config.

Original comment by crazy4ch...@gmail.com on 25 Mar 2013 at 11:27

GoogleCodeExporter commented 9 years ago
I would leave only one sample custom function, maybe a more interesting one 
(capitalize? leet_text?), and then add a single call to function_exist(). We 
can improve the comment if it looks unclear, but it's an advanced feature that 
php-ignorant users will probably ignore anyway.

//a list of custom functions that can be applied to columns in the databases
//make sure to define every function below if it is not a core PHP function
$custom_functions = array('md5', 'sha1', 'time', 'strtotime', 'leet_text');

//define all the non-core custom functions
if (!function_exists('leet_text')) {
  function leet_text($value)
  {
    return str_replace(...);
  }
}

Original comment by dreadnaut on 26 Mar 2013 at 7:17

GoogleCodeExporter commented 9 years ago
Hmm. But still, I think the function_exists() approach has a real problem: It 
will use the function defined in phpliteadmin.php. So if somebody changes the 
function in phpliteadmin.config.php, his changes will have no effect.

I have another idea: We can just out-comment the function. People who are 
capable of writing a php-function will know what to do ;-)

//define all the non-core custom functions
/* This is an example function. To use it, remove the comments around it. 
function leet_text($value)
  {
    return str_replace(...);
  }
*/

But one small problem then would remains: Even lots of users might not add 
custom functions, some might use the example functions that are kind of 
built-in.
Okay. Looking at them, probably nobody uses them :-D

Original comment by crazy4ch...@gmail.com on 26 Mar 2013 at 7:25

GoogleCodeExporter commented 9 years ago
Ah, comments! The simplest solution is actually the best :)

We should also comment the identifier in the array above

//a list of custom functions that can be applied to columns in the databases
//make sure to define every function below if it is not a core PHP function
$custom_functions = array(
  'md5', 'sha1', 'time', 'strtotime',
  // add the names of your custom functions here!
  /* 'leet_text', */
);

// define your custom functions here!
/*
function leet_text($value)
{
  return str_replace(...);
}
*/

Original comment by dreadnaut on 27 Mar 2013 at 1:24

GoogleCodeExporter commented 9 years ago
Here's a patch equivalent to my previous post.

Original comment by dreadnaut on 29 Mar 2013 at 12:34

Attachments:

GoogleCodeExporter commented 9 years ago
Looks good. Commit it.

Original comment by crazy4ch...@gmail.com on 2 Apr 2013 at 10:40

GoogleCodeExporter commented 9 years ago
Done, now in the repository as r396.

Original comment by dreadnaut on 3 Apr 2013 at 11:51