We need to rip out all the .info file handling code to make room for a more useful PHP based file handling.
See meta issue #897822: [META ISSUE] Stop storing skins in .info files; implement skins as PHP instead for more information.

CommentFileSizeAuthor
#9 skinr_rip_info.9.patch13.82 KBmoonray
#7 skinr_rip_info.6.patch13.84 KBmoonray
#5 skinr.rip-info.5.patch14.33 KBsun
#4 skinr_rip_info.patch13.07 KBmoonray
#1 skinr_abstract_info.patch29.15 KBmoonray

Comments

moonray’s picture

moonray commented
StatusFileSize
new skinr_abstract_info.patch29.15 KB

This patch does not change any code (other than including the new include file). All it does is abstract all functions that relate to .info file handling into a separate include file. Hopefully this will make it easier to work on this issue. We can also start cleaning up skinr.module and so it makes minimal calls to the functions in this include.

The following functions are referenced from files outside the .info handling include:
skinr_rebuild_skinset_data()
skinr_ui.admin.inc

skinr_skinsets()
skinr.module [in function: skinr_skin_data()]
skinr_ui.admin.inc [in functions: skinr_ui_admin_skinsets_submit() | skinr_ui_admin_skinsets_settings()]
skinr_ui.module [in function: skinr_ui_menu()]

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Active » Needs work

Thanks!

It technically makes no sense to keep that .info file handling code. All it really does is complicating the transition.

So what we need here is a patch that hopefully consists of deleted lines only. Merely retaining the "skinsets" functionality.

moonray’s picture

moonray commented

It's not that simple since it's too much interwoven. This include is all the file loading and parsing code; that's all that really needs cleaning up. The rest of skinr.module should probably not be touched much.

The skinsets functionality is essentially duplicating the theme's .info file (with some minor alterations). So we're not really getting rid of much in the end.

moonray’s picture

moonray commented
Status: Needs work » Needs review
StatusFileSize
new skinr_rip_info.patch13.07 KB

OK, here's a patch that doesn't try to abstract file handling functionality (I guess we can do that down the line; this way the patch is easy to read).

I believe this strips out all of the theme's .info functionality, and leaves the separate skin (or skinset) functionality in place. Note that this patch does not yet convert anything to use PHP include files for skins.

EDIT: Note also that this will break any skins for themes that include skin information in their .info file.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
StatusFileSize
new skinr.rip-info.5.patch14.33 KB

mmm, rather like this.

Status: Needs review » Needs work

The last submitted patch, skinr.rip-info.5.patch, failed testing.

moonray’s picture

moonray commented
Status: Needs work » Needs review
StatusFileSize
new skinr_rip_info.6.patch13.84 KB

Fair enough. But then let's fix that error that occurs if no skinsets are found.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work

Thanks!

+++ skinr.module
@@ -811,50 +742,40 @@ function _skinr_add_path_to_files($files, $path, $type) {
+    dpm($skinsets);

I have the time impression you should write yourself a script to grep for dsm(), dpm(), and debug()? :-D

Powered by Dreditor.

moonray’s picture

moonray commented
Status: Needs work » Needs review
StatusFileSize
new skinr_rip_info.9.patch13.82 KB

Awh man. Seriously? I left ANOTHER in there?
I was actually just talking in the #skinr channel about writing that script. :p

Here's the patch without dpm.

jacine’s picture

jacine
she/her
Location New York
commented
Status: Needs review » Fixed

Committed :) http://drupal.org/cvs?commit=443546

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.