I'm not sure yet myself it this is a bug or not, but in case you choose to remove all files, possible leftovers could be in the directory.
EG, the first time, I exported everything and got a demo.pipelines.inc file. The second time, I used the selection mode and removed the pipelines exportables, but the file was still there. Should we remove this or not, that's the question now.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

damiankloip’s picture

I would say that at least as a default if the module is being overwritten the file should be removed. It should be treated as a new module I think. We could also add another --option to the command for other use cases?

swentel’s picture

Yeah, default should remove all, I'll post a patch soon.

damiankloip’s picture

Nice! :) In the CTools queue? Are we going to go with an additional option too? I think that is a good plan.

swentel’s picture

Hmm, not sure about the queue, in a way, I like the fact that we can discuss an experiment here without disturbing merlin with dozens of emails :)
The option is a good idea, I'll put that in as well. Let me know what you prefer re: the queue, it's all the same for me actually.

damiankloip’s picture

That's a good point, maybe here is a good testbed for functionality. We can play around a bit more :) The "No pengiuns were harmed" patch is on the ctools 7.x-dev branch!

swentel’s picture

Status: Active » Needs review
FileSize
2.97 KB

This should do it.

damiankloip’s picture

FileSize
2.96 KB

Does the logic need to be more like this? Otherwise, I'm confsued :) See in this patch too - Shall we use the spl DirectoryIterator instead?

swentel’s picture

Status: Needs review » Needs work

Hmm, you're right. Like the spl DirectoryIterator as well, much cleaner than using strpos. Couple of nitpicks:

+++ b/drush/ctools.drush.incundefined
@@ -154,18 +156,29 @@ function drush_ctools_export($module = 'foo') {
+  $overwrite = drush_get_option('overwrite', FALSE);

After some thinking, it probably makes more sense to rename the variable to something like 'remove', no ? Because, it used to overwrite, now, it first removes them all and creates them again.

+++ b/drush/ctools.drush.incundefined
@@ -154,18 +156,29 @@ function drush_ctools_export($module = 'foo') {
+      if (drush_confirm(dt('All files except for the .module file will be removed. You can choose later if you want to overwrite that file as well. Are you sure you want to proceed ?', array('!module' => $module)))) {

No !module placeholder in the string.

Other than that, I like!

damiankloip’s picture

Cool, I think we are agreed then. I will re roll a patch with the amends/fixes from #8. Shall we change the name of the option from --overwrite to --remove too?

swentel’s picture

Yes, all overwrite to remove should be fine.

damiankloip’s picture

FileSize
2.97 KB

Ok, agreed on this patch?

swentel’s picture

+++ b/drush/ctools.drush.incundefined
@@ -19,11 +19,13 @@ function ctools_drush_command() {
+      'drush ctex export_module --subdir=exports --overwrite=1' => 'Same as above, but automatically removing all files, except for the .module file.',

overwrite should be remove.

Other than that, looks good.

damiankloip’s picture

FileSize
2.98 KB

But sticking to the _drush_ctools format of function names...

damiankloip’s picture

bah! You are right. One sec :)

damiankloip’s picture

FileSize
2.97 KB

maybe now?

damiankloip’s picture

FileSize
2.97 KB

OK, This is THE one :)

damiankloip’s picture

FileSize
2.97 KB

With logic fixed, for file unlinking (my fault!)

swentel’s picture

Status: Needs work » Reviewed & tested by the community

YES!

damiankloip’s picture

Project: » Chaos Tool Suite (ctools)
Version: » 7.x-1.x-dev
Assigned: Unassigned » damiankloip
damiankloip’s picture

Status: Reviewed & tested by the community » Fixed

Committed to 7.x-1.x

Status: Fixed » Closed (fixed)

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