Support from Acquia helps fund testing for Drupal Acquia logo

Comments

willvincent’s picture

Status: Active » Needs review
FileSize
1.29 KB

There are a handful of examples around the net on using glob() recursively, but none of them seem to also support patterns.

Attached is a patch that adds this recursive functionality by using the RecursiveIteratorIterator class, and building up a list of all subdirectories within the provided directory. Then the files list is built up by running glob() against each of those, and additionally removing directories from the files list, so that the importer won't try (and fail) to import a directory as a file.

willvincent’s picture

This patch is slightly better, previous patch in certain cases would not include everything (including the starting directory)

willvincent’s picture

Ack.. it's one of those days. Removed the extra if that's not needed... this one should be good to go.

Status: Needs review » Needs work

The last submitted patch, include_recursion_into_subdirectories_on_import-1902586-3.patch, failed testing.

willvincent’s picture

Status: Needs work » Needs review
FileSize
1.19 KB

I could have sworn it wasn't a Monday.. Not sure what happened there, this one should apply

willvincent’s picture

Ok.. one last one (hopefully). This is a cleaner method of removing the subdirectories from the file list

queenvictoria’s picture

I've tested the patch in #6 and unfortunately it doesn't work for me with multiple patterns eg: *.jpg|*.png

I'd not discovered this issue before working on it myself (foolishly I'd looked in the file_entity issue queue) so I attach my version as well. It's nearly the same as @willvincent s except it provides a 'recurse' checkbox on the form and uses a regular expression to check for pattern matched files.

willvincent’s picture

That's a much nicer solution.

Dave Reid’s picture

Status: Needs review » Needs work

We just changed the module to use file_scan_directory() now so this needs to be re-rolled.

queenvictoria’s picture

Status: Needs work » Needs review
FileSize
1.95 KB

Re-rolled against HEAD

aaron’s picture

Status: Needs review » Needs work

This works fairly well. However, the regex pattern could use an "i" on the end of it, so that it will ignore the case of anything in the pattern. Additionally, I believe that the description on the check box should make it clear that we will iterate through subdirectories.

aaron’s picture

Status: Needs work » Needs review
FileSize
2 KB

This patch addresses those concerns.

Dave Reid’s picture

Wasn't the previous code also case-sensitive?

aaron’s picture

yes. should I make a new issue?

30equals’s picture

Hey all,

Since i needed this functionality i decided to pick up this issue and to provide a new patch based on #12.
Tested it locally without a glitch, so testbot, do your thing.

emerham’s picture

joseph.olstad’s picture

Nice, this is a great feature , I am going to try out this patch myself as I also have a use for this.

joseph.olstad’s picture

Assigned: Unassigned » joseph.olstad

marking assigned to me, I'll try to test this and review this when I have a few spare cycles. Meanwhile, others please feel free to review and provide feedback.

emerham’s picture

so I used it on two sites so far, and it worked with out issues, I tried to make an interdiff and got errors, but I didn't change any of the code from #1902586-15: Support recursion into subdirectories for file import, just updated the position to what is in 2.x-dev.

  • joseph.olstad committed b1e1b29 on 7.x-3.x authored by emerham
    Issue #1902586 by willvincent, queenvictoria, aaron, emerham, 30equals:...

  • joseph.olstad committed b1e1b29 on 7.x-2.x authored by emerham
    Issue #1902586 by willvincent, queenvictoria, aaron, emerham, 30equals:...
joseph.olstad’s picture

Status: Needs review » Fixed

reviewed this, it looks good, and seeing as this recursive import is optional, it should work for everyone.

joseph.olstad’s picture

too bad this didn't make it into 7.x-2.14 , we just tagged and released 2.14 a couple days ago.

however it will make it into 7.x-2.15 whenever that will be.

Status: Fixed » Closed (fixed)

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