the attached is a version of the xml based form generator with one bug fixed, and wider xslt engine support. specifically, it supports both xsltproc and xalan.
commit 1: your original code
commit 2: removed broken forms and/or forms that touch core openemr stuff
commit 3: created/installed a working form for test/review
Over next several days, will put a review on the code in github.
(quick aside, please start to learn git/github; makes collaboration far easier; ie. .the time I spent putting this together on git/github could of gone towards the actual review)
thanks for the contribution,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Done with code review/testing. See my comments in github at end of commit 1 and within commit 3. Another question I have is what the scripts in xmlformthrough are used for?
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
the scripts for xmlformthrough/ are kindof an experiment, going in the direction of complete integration.
If you replace any of the .PHP files in one of the generated forms with the ones in xmlformthrough, touch up one path in the files (still working on generating that part), and place your .xml file in just the right spot...
then openemr generates the PHP for each page on-the-fly, then executes them. so, no generated .php files on disk.
No, i don't intend for anyone sane to run that in production. but when i'm editing a forrm's XML live to "get the look right", its real nice to not even have a 'compile' step.
as for github, I'm actually becoming very familiar with the git development style. i'm just gitorious centric, and would prefer to continue to learn gitorious, since i can have the source code to it. ;)
I do like the inline commenting, that's neat. i wonder if i can do that on gitorious...
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Oh, and regarding history.xml.. it does not attempt to replace. it effectively acomplishes replacement in trees before the 'new tab ui'.
I would like to take this in the direction not just of adding forms to our current system, but of removing code from having to be maintained in dozens of places. A year ago, the output of history.xml was a drop-in replacement for the history code in openemr.
I had to go through backflips to make that one work, as history contains data objects that are not used anywhere else in the system. XMLFORMGEN supports those objects. With your continued cooperation, i would like to target complete replacement of the history subsystem.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Hey Brady, I'm going through your first comment, and making the change you recommend includes a ton of script content directly into my xhtml output.
do you know why this sript must be included in our forms, or some easy way to make this go away? I spend a lot of time staring at the source of these, and it would be very nice to not include script content directly into forms if avoidable.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
here's a version with all of the updates from commit 038ed26bd38d61f3..
If you look at the source in the web browser of view.php or new.php, you'll find a large section of script being spit out by dynarch_calendar_en.inc.php.
as a general rule, i like to see script content in a separate URI, not in the xhtml document itsself.
I'm using gitorious to publish my other work would you like me to go ahead and start pushing this up, instead of dropping tarballs? ;)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Gitorious is better tarballs (much easier to grab your commits of your repo than have to deal with tarballs). Problem with gitorious is no way to embed review comments, but still better than tarballs.
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Regarding script stuff, that is only way (that I know of) to get the javascript calendar internationalized (by running it through the php engine, can then use the xl() function to internationalize javascript code).
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Still need to finish replacing all the dynarch_calendar_en.js lines, htmlentities functions. innodb database thing, and deal with the signatures tables issues brought up.
I suggest providing a simple example xml form (without signature feature and can discuss how to get a global signature table into openemr codebase on the forums as another issue). then if you fix above issues and it tests ok, it will be ready for commit. Then you begin getting more ambitious; goal now should be to make a simple form correctly and get your code committed.
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
hey,
Also created a github branch to focus on your signature method. If this works, then we should place the table directly in OpenEMR so it is stable (otherwise run risk of having incompatible tables if have it created by the form generator): http://github.com/bradymiller/openemr/commits/form_signature_mathod
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Please describe the method there. Ideally, we could then commit the form generator code and the signatures table in the same commit. I'll package it up; you just need to convince the community why the signatures table/mechanism is useful.
thanks,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
To avoid this project stalling out again, I committed the below proposed rebased code. I also removed the commented out signature line in the example since this signature table/method is still under discussion in the forums. If the signature method makes sense, then should include the table in database.sql and upgrade script to ensure it's stable and can be modified from a central location (otherwise if you only modify it in your xmlformgen stuff then you'll start having forms that are no longer compatible in the future).
So getting the signature table/method into the codebase would be the next logical step here (also would be cool to get your on the fly stuff working and in also...)
thanks,
-brady
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
hi,
I put your code here (for review/testing):
http://github.com/bradymiller/openemr/commits/xmlformgen
commit 1: your original code
commit 2: removed broken forms and/or forms that touch core openemr stuff
commit 3: created/installed a working form for test/review
Over next several days, will put a review on the code in github.
(quick aside, please start to learn git/github; makes collaboration far easier; ie. .the time I spent putting this together on git/github could of gone towards the actual review)
thanks for the contribution,
-brady
hey,
Done with code review/testing. See my comments in github at end of commit 1 and within commit 3. Another question I have is what the scripts in xmlformthrough are used for?
-brady
Brady,
the scripts for xmlformthrough/ are kindof an experiment, going in the direction of complete integration.
If you replace any of the .PHP files in one of the generated forms with the ones in xmlformthrough, touch up one path in the files (still working on generating that part), and place your .xml file in just the right spot...
then openemr generates the PHP for each page on-the-fly, then executes them. so, no generated .php files on disk.
No, i don't intend for anyone sane to run that in production. but when i'm editing a forrm's XML live to "get the look right", its real nice to not even have a 'compile' step.
as for github, I'm actually becoming very familiar with the git development style. i'm just gitorious centric, and would prefer to continue to learn gitorious, since i can have the source code to it. ;)
I do like the inline commenting, that's neat. i wonder if i can do that on gitorious...
Oh, and regarding history.xml.. it does not attempt to replace. it effectively acomplishes replacement in trees before the 'new tab ui'.
I would like to take this in the direction not just of adding forms to our current system, but of removing code from having to be maintained in dozens of places. A year ago, the output of history.xml was a drop-in replacement for the history code in openemr.
I had to go through backflips to make that one work, as history contains data objects that are not used anywhere else in the system. XMLFORMGEN supports those objects. With your continued cooperation, i would like to target complete replacement of the history subsystem.
Hey Brady, I'm going through your first comment, and making the change you recommend includes a ton of script content directly into my xhtml output.
do you know why this sript must be included in our forms, or some easy way to make this go away? I spend a lot of time staring at the source of these, and it would be very nice to not include script content directly into forms if avoidable.
hi,
Not sure I understand what you mean. What script?
thanks,
-brady
brady,
here's a version with all of the updates from commit 038ed26bd38d61f3..
If you look at the source in the web browser of view.php or new.php, you'll find a large section of script being spit out by dynarch_calendar_en.inc.php.
as a general rule, i like to see script content in a separate URI, not in the xhtml document itsself.
I'm using gitorious to publish my other work would you like me to go ahead and start pushing this up, instead of dropping tarballs? ;)
hi,
Gitorious is better tarballs (much easier to grab your commits of your repo than have to deal with tarballs). Problem with gitorious is no way to embed review comments, but still better than tarballs.
-brady
hi,
I'll wait for you to get it up to gitorious.
Regarding script stuff, that is only way (that I know of) to get the javascript calendar internationalized (by running it through the php engine, can then use the xl() function to internationalize javascript code).
-brady
hey,
Had a little time, so looked at your tarball and put here (last commit on that branch):
https://github.com/bradymiller/openemr/commits/xmlformgen_rev2
Please go through all the original embedded comments in original review here (search through page for bradymiller to find them):
http://github.com/bradymiller/openemr/commit/244ea5819750cd1fba82cbeedb6b995ee9c0c423
Still need to finish replacing all the dynarch_calendar_en.js lines, htmlentities functions. innodb database thing, and deal with the signatures tables issues brought up.
I suggest providing a simple example xml form (without signature feature and can discuss how to get a global signature table into openemr codebase on the forums as another issue). then if you fix above issues and it tests ok, it will be ready for commit. Then you begin getting more ambitious; goal now should be to make a simple form correctly and get your code committed.
-brady
Here is a new version, with a simpler communication_log example, that should match the 'normal' behavior of an openemr form.
I think I touched on all the issues in your review, though i don't know what you're referring to regarding PHP6.
new xml form generator
hey,
I put your original commit, along with my changes to get it ready for commit to sourceforge:
http://github.com/bradymiller/openemr/commits/xmlformgen_rev3
Here's a rebase of above that plan to commit to sourceforge:
http://github.com/bradymiller/openemr/commit/ab7eadef04b4e693c3cb8003bd45afa155001f7b
-brady
hey,
Fixed your author info in the rebase with planned new commit to sourceforge here:
http://github.com/bradymiller/openemr/commit/895b36f7a2f879e3bd389af2e7d0c4fbb9c1fe25
-brady
hey,
Also created a github branch to focus on your signature method. If this works, then we should place the table directly in OpenEMR so it is stable (otherwise run risk of having incompatible tables if have it created by the form generator):
http://github.com/bradymiller/openemr/commits/form_signature_mathod
-brady
Julia,
I posted a thread to discuss your signature table/method:
http://sourceforge.net/projects/openemr/forums/forum/202506/topic/4079752
Please describe the method there. Ideally, we could then commit the form generator code and the signatures table in the same commit. I'll package it up; you just need to convince the community why the signatures table/mechanism is useful.
thanks,
-brady
hey,
To avoid this project stalling out again, I committed the below proposed rebased code. I also removed the commented out signature line in the example since this signature table/method is still under discussion in the forums. If the signature method makes sense, then should include the table in database.sql and upgrade script to ensure it's stable and can be modified from a central location (otherwise if you only modify it in your xmlformgen stuff then you'll start having forms that are no longer compatible in the future).
So getting the signature table/method into the codebase would be the next logical step here (also would be cool to get your on the fly stuff working and in also...)
thanks,
-brady