Repository navigation
Implement wp media import command - #367
Conversation
|
Looks pretty solid. |
There was a problem hiding this comment.
Wouldn't it make more sense to rename this to --title?
There was a problem hiding this comment.
Probably so. I was just going off the argument names in media_handle_sideload(). It would make more sense, though, to use the terms that are used in the media editor: title / caption / alt text / description. I'll revise.
|
The arguments do make more sense when following more closely to the labels used in the admin media uploader. Fixed that. I reverted the change I made to file completions for this command, because it was breaking file globbing. Completions will have to be refactored anyways, as mentioned in #212. I'll add a couple behat features later today or tomorrow to test this: probably one importing from a local file and one importing from a URL. It will cause a merge conflict with #366 though, so will need to be rebased once that's merged in. |
|
I see you added a test for the command, but it currently fails on Travis. Another thing I noticed is that the success message is kind of redundant: It should be: and I think it should point to the original URL, not the temporary path. |
Introduces `wp media import` command to sideload images and import them as attachments. Needs tests, second opinion, etc.
If a recognizable scheme is passed in the <file> argument, `wp media import` will attempt to download the file to a temp directory before importing it.
Filtering bash completions breaks bash's default filename globbing, which is a more useful feature here.
Use the same field labels as are used by the admin media uploader, to avoid confusion: title / caption / alt / description; rather than the variable names used internally, where 'description' is really title and so on.
This flag will set the uploaded image to be the post thumbnail of the post its attached to. Also, fix the success message and the Behat feature, so that tests don't fail on environments that use a different tmp path.
Add option description for `--featured_image`, and briefly explain examples
|
I just ran this and was surprised to find that the original files disappeared. I was expecting the command to copy them instead. |
|
Whoa! I'm surprised by that too, it wasn't the intended behavior. I'll add some more tests, and then see why that's happening. |
Apparently `wp_handle_sideload` performs a`rename()` operation on the file it's sideloading. In this case thats an unexpected side effect - you don't want the original image to go away when importing it to WordPress.
One test for failure on trying to import non-existant image, and one to make sure that local images are not removed on import
|
OK, that issue should be fixed, and seems to be confirmed as fixed by the tests. I didn't realize the |
There was a problem hiding this comment.
I don't like that you don't see what command is being run. You should be able to write:
When I run `wp media import {DOWNLOADED_IMAGE}`
Rewrote the feature step so that it displays the actual command being run, and uses variable substitution. * I had to modify the definition of the "file should exist" step to allow variable substitution. All of the existing tests still pass, but I'm not sure if this is best practice. I can write a new step instead, if that makes more sense.
|
Good comment on using variables in the feature definitions, that definitely makes them easier to read. In order to get the "file should exist" step to replace variables and work with a full path rather than just the filename, I had to redefine that step a bit. All of the existing features still pass, but I'm not sure whether it would be better to define a new step anyways? If so, let me know and I'll rewrite this. |
|
No, it looks good. Going in. |
Implement `wp media import` command
I needed a quick way to import images into a WP site, so this is what I
came up with. It takes a pathname, a list of file paths, or any system-globbed
file path, and calls
media_handle_sideloadon any files found.Needs tests, I know - I haven't looked through Behat, but I'll figure out
how to write some basic tests to make sure that it works.
Does this seem useful? Any thoughts on the implementation?