revig / revigniter

Framework for people who build websites using LiveCode
https://revigniter.com/
Apache License 2.0
28 stars 9 forks source link

including config files causes execution error because of local variable redeclaration #3

Closed montegoulding closed 8 years ago

montegoulding commented 9 years ago

I just ran into an issue where I wanted to include the db config in order to call a handler to set the db name to connect to prior to loading the database rather than using the db group name. I think just about all the instances of include in revIgniter need to be replaced with require. At least those with variable declarations which is probably all of them. That way they will only ever be included once.

I'm happy to submit a PR on this but I thought it would be good to discuss first.

revig commented 9 years ago

Monte, what you describe is a very special case. I would have never thought anyone would come up with the idea of doing that, especially if you consider that the connection group can be easily defined using the first parameter of rigLoadDatabse. Of Course I am sure you have your reasons.

Nevertheless, I checked all include statements and you are right, (at first glance) almost all of them can be safely replaced. An exception would be for example the loading of DBdriver.lc. Admittedly this is a rare case, but let's assume that I'd like to connect to a MySQL database followed by a connection to a PostgreSQL db. Now if DBdriver.lc was required in DB.lc, instead of included, the settings in DBdriver.lc would not be changed as needed.

As I wrote this is a rare case and actually currently this is disabled by using require to load DB.lc because it is still not tested. This means as long as DB.lc is "required" I can replace the include statement to load DBdriver.lc too, not to mention the configuration file database.lc of course.

Anyway, no need to submit a PR. I will take charge of the subject and will publish an appropriate update shortly. Thanks a lot for your support.

montegoulding commented 9 years ago

Thanks Ralf, yes a rather special use case but nonetheless it may be an issue someone else bumps into a different way.