At the moment, Inkribbon is probably the only theme with Drush support (not sure). It wouldn’t suprise me if Zen or other popular base themes would start supporting it in the near future, though.

Drush doesn’t look in sites/all/themes for *.drush.inc files by default, so people need to change their config to be able to use commands provided by themes.

Would it be a good idea to add sites/all/themes to the list of default directories?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Priority: Minor » Normal

"Create a new Inkribbon sub theme.". Nice command!

Yes, we should add themes to the search path for commands.

jasonn1234’s picture

Status: Active » Needs review

FYI - This feature is patched here: #537280: D7 compatibility

EDIT: ^^ *not* see next comment for patch.

jasonn1234’s picture

FileSize
1.06 KB

Patch attached. With this patch applied I was able to see the command with "drush help" and create a couple of inkribbon subthemes.

There might be some additional work required. Honestly - I don't really understand all the dependencies / possible conflicts / issues related to this, but I thought I'd submit what I came up with just to solicit feedback and keep the ball rolling.

From irc:

...
Vertice: i think the addition of the search path is out of scope for that
Vertice: because of the inheritence angle
Vertice: what happens if you want to clone a cloned zen theme
Vertice: etc
Vertice: i'm not saying it's a bad idea
....
moshe weitzman’s picture

Status: Needs review » Needs work

system_theme_data() does not exist in D7. needs to support all d5/d6/d7 ... i'm not too worried about duplicate drush files/commands coming from theme. they will be true dupes so it is OK that they trample on each other.

jonhattan’s picture

`drush_get_modules()` and `drush_get_themes()` were recently introduced by `port to D7` patch.

moshe weitzman’s picture

pretty trivial now. anyone care to reroll?

jonhattan’s picture

FileSize
1.5 KB

list_themes() return all available themes unlike module_list() that do only for enabled ones. So I switched to use drush_get_projects() and explicitly check status.

jonhattan’s picture

Status: Needs work » Needs review
moshe weitzman’s picture

Status: Needs review » Fixed

Committed. Thx.

jonhattan’s picture

Status: Fixed » Needs review

There're several reasons to reverse this patch:

Author of Inkribbon has told me he dropped drush support. So there's no theme with drush support at present afaik.

In addition I can't think of distinct use case for a theme command than creating a subtheme. Perhaps I'm not imaginative enough but actually they're not already coded out there....

If the only use case is creating a subtheme,... if there is a general algorithm for subtheming... it may be created for drush core or as a contrib module.
In case there's no general algorithm and a theme provide its own command... in this case it shouldn't be necessary to enable a theme in order to create a subtheme...so themers could instruct users to move a file to ~.drush

And last but most important, the solution provided in #7 is based on a unnecessary overhead. To solve the issue it is only needed a list of names of enabled modules and themes. There're faster ways to obtain this than calling drush_get_projects() (that does a rebuild of system table) and iterating over it.

So I bet for reversing the patch as I think commands for themes is unnecessary. If you feel this necessary, I can provide a more efficient patch if you prefer.

moshe weitzman’s picture

I'm good with reverting this. If anyone objects, please speak up soon.

greg.1.anderson’s picture

There are already a couple contrib modules for creating subthemes, so I'm in agreement.

moshe weitzman’s picture

Status: Needs review » Fixed

rolled back.

Status: Fixed » Closed (fixed)

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

JohnAlbin’s picture

Status: Closed (fixed) » Needs work

In case there's no general algorithm and a theme provide its own command... in this case it shouldn't be necessary to enable a theme in order to create a subtheme...so themers could instruct users to move a file to ~.drush

There is not general algorithm for sub-theme generation. And it isn't necessary to enable a theme in order to create sub-themes. And I'm missing the logical conclusion part of “so themers could instruct users to move a file to ~.drush.” Why?

The problem with having themers move a file to ~/.drush is it requires the theme developer to maintain drush commands that will work with all past and future versions of their theme. Zen just opened up a 4th major supported branch (2 in D6, 2 in D7) and I'd rather not deal with support problems because someone is using the wrong version of the drush commands. :-p

I don't see a reason why any installed theme can't have live drush commands. Drupal core already runs the code of disabled base themes of enabled sub-themes.

greg.1.anderson’s picture

Hi John, thank you for your comments. If you would like to support zen subtheme creation via drush in zen, I for one am strongly in favor of supporting it. Part of the background for the discussion above is that _drush_find_commandfiles recently changed so that only enabled modules are searched for .drush files. This made things work a lot better in several instances--for modules, at least. For example, by not including disabled modules, it meant that the sanitize options for disabled modules would not be added to the list of options in sql-sync -- and there are other examples where running the drush hooks would just do the wrong thing (their code would not run) if their associated module was disabled.

It seems to me that all we would need to do to support this is add:

        $searchpath[] = 'sites/all/themes';

to DRUSH_BOOTSTRAP_DRUPAL_SITE outside of the if ($phase_max < DRUSH_BOOTSTRAP_DRUPAL_FULL). Then we'd get theme .drush.inc files at bootstrap_site and higher for all themes, disabled or not. Most users should not have too many themes installed, so I don't think there would be a big performance hit for including them all. I also agree with your point that themes behave differently than modules, and there would be no harm in including them all. I don't expect that theme .drush files would attempt to hook drush in the same way that a module .drush file would, so I think this should be safe enough.

greg.1.anderson’s picture

Status: Needs work » Needs review
FileSize
894 bytes

This works well for me, and is very simple. If there are no issues with it, it would be good to commit to HEAD and to the upcoming 4.3 release.

moshe weitzman’s picture

I'd like for jonhattan to review this as he wanted to roll back the original patch.

greg.1.anderson’s picture

I agree that jonhattan should review, but I think that the issue with #7 was with drush_get_projects in BOOTSTRAP_FULL, which I do not think is necessary.

One thing that #7 does that I do not do in #17 is include $searchpath[] = conf_path() . '/themes'; in BOOTSTRAP_SITE. This allows Drupal core themes to provide drush.inc files; I am neutral about putting that back in.

jonhattan’s picture

Status: Needs review » Reviewed & tested by the community

Greg's approach is different and lightweight. I'm aligned with #16.

jonhattan’s picture

For clarification, a quote from #10:

the solution provided in #7 is based on a unnecessary overhead. To solve the issue it is only needed a list of names of enabled modules and themes. There're faster ways to obtain this than calling drush_get_projects() (that does a rebuild of system table) and iterating over it.

So I bet for reversing the patch as I think commands for themes is unnecessary. If you feel this necessary, I can provide a more efficient patch if you prefer.

greg.1.anderson’s picture

Status: Reviewed & tested by the community » Fixed

Committed; thanks!

greg.1.anderson’s picture

Assigned: Unassigned » msonnabaum
Status: Fixed » Patch (to be ported)

Oh yes, I suggest that we also put this back in 4.x so that zen can use it in the not-too-distant future.

danillonunes’s picture

#17 sounds great, but it will works with themes living in /sites/exemple.com/themes too?

greg.1.anderson’s picture

Status: Patch (to be ported) » Needs work

Oh, yes, I misspoke in #19; conf_path() is sites/example.com, so we should probably add that one back in too.

greg.1.anderson’s picture

Version: » All-versions-4.x-dev
Status: Needs work » Patch (to be ported)

Committed http://drupalcode.org/project/drush.git/commit/a294614 to master; would be good to get this in drush-4.3 too.

JohnAlbin’s picture

Status: Patch (to be ported) » Needs review
FileSize
564 bytes

Here's a patch that applies to the 7.x-4.x branch. That's the right branch for 4.x, yes?

I've also committed a zen.drush.inc file to Zen's 7.x-3.x branch. So you can test this patch against the latest Zen 7.x-3.x-dev release.

jonhattan’s picture

Status: Needs review » Patch (to be ported)

Thanks John for the patch, yes, 7.x-4.x is the branch.

Patches to 4.x usually get commited when we plan a new release (Mark is the maintainer for 4.x). Issue version, status and assignment were ok.

msonnabaum’s picture

Status: Patch (to be ported) » Fixed

Backported to 4.

JohnAlbin’s picture

yayz! :-D

Status: Fixed » Closed (fixed)

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