Posted by zoo33 on August 26, 2010 at 9:05am
| Project: | FFmpeg Converter |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | task |
| Priority: | normal |
| Assigned: | zoo33 |
| Status: | closed (fixed) |
Issue Summary
This patch basically rips out the preset admin code and instead implements export_ui in Ctools. The result is about 470 deleted and 180 added lines of code:
$ grep -c "^\-" ffmpeg_converter_export_ui.patch
471
$ grep -c "^\+" ffmpeg_converter_export_ui.patch
183It seems to work fine, but testers and reviewers are warmly welcome. I'll probably commit this very soon in order to reach more testers through the -dev version.
Comments
#1
Aargh, the patch file!
#2
...and the status.
#3
+++ contributions/modules/ffmpeg_converter/plugins/export_ui/ffmpeg_converter_ctools_export_ui.inc Locally New@@ -0,0 +1,97 @@
+ // Check this preset's export type/storage.
+ if (isset($preset->export_type)) {
+ $form['intro'] = array('#weight' => -1);
+ switch ($preset->export_type) {
+ case 1:
+ // "Normal" type.
+ $form['intro']['#value'] = t('<strong>Status: normal</strong><br />This preset is defined in the database.');
+ break;
+ case 2:
+ // "Default" type.
+ $form['intro']['#value'] = t('<strong>Status: default</strong><br />This preset is defined in code.');
+ break;
+ case 3:
+ // "Overridden" type.
+ $form['intro']['#value'] = t('<strong>Status: overridden</strong><br />This preset is defined in code and overridden in the database.');
+ break;
+ }
+ }
I think this code if exists, should move to CTools itself. Other stuff from a quick eye-review look ok, but not really tested.
Powered by Dreditor.
#4
Maybe those lines weren't actually that useful in the first place. You can clearly see the status of each preset on the overview page, so I don't think you need to be reminded of that on each editing form.
Attaching a new version of the patch.
#5
Here's another version where I've cleaned up some of the code that loads presets. I now tap into Ctools' load mechanism in order to be able to sanitize presets before they are used anywhere.
#6
...aaaand the patch.
#7
Oh, a Varnish error there... Trying again.
#8
Committed! Thanks Amitaibu for the initial idea, the review – and the amazing presentation at Drupalcon!
#9
Automatically closed -- issue fixed for 2 weeks with no activity.