allow user to specify frame_index for generating thumbnails from videos or pdfs #2155
Conversation
| @animated = options.fetch(:animated, true) | ||
| @auto_orient = options.fetch(:auto_orient, true) | ||
| if @auto_orient && @current_geometry.respond_to?(:auto_orient) | ||
| @current_geometry.auto_orient | ||
| end | ||
|
|
|
I like this. Can you please address Hound comments, while we are on it? Also, we can achieve the initialization of the default value with Thanks for your work and contribution! |
…e_index in identified_as_animated?, set default value of frame_index using options.fetch
| @@ -118,4 +119,4 @@ def identified_as_animated? | |||
| raise Paperclip::Errors::CommandNotFoundError.new("Could not run the `identify` command. Please install ImageMagick.") | |||
| end | |||
| end | |||
| end | |||
| end | |||
There was a problem hiding this comment.
I'll fix this in an update, i removed the white space in the wrong part of the file
| @animated = options.fetch(:animated, true) | ||
| @auto_orient = options.fetch(:auto_orient, true) | ||
| if @auto_orient && @current_geometry.respond_to?(:auto_orient) | ||
| @current_geometry.auto_orient | ||
| end | ||
|
|
||
| :source => "#{File.expand_path(src.path)}#{'[' + @frame_index.to_s + ']' unless animated?}", | ||
| :dest => File.expand_path(dst.path) | ||
| ) | ||
|
|
|
Looking good! Can you please add a regression test for this? Also, can you address the new Hound comments? Thank you very much for sharing your work! 😃 |
|
i'll try to wrap all of this up this week. been busy launching this new product |
|
@jacobbullock congrats on your launch! And don't forget your good PR! :) |
| success = convert(parameters, | ||
| :source => "#{File.expand_path(src.path)}#{'[' + @frame_index.to_s + ']' unless animated?}", | ||
| :dest => File.expand_path(dst.path), | ||
| ) |
There was a problem hiding this comment.
@tute do you what they want here for alignment?
|
@tute should I add the test in spec/thumbnail_spec.rb? |
|
New tests in |
|
ping @jacobbullock. Thank you! |
…against multi-frame file formats before allow a specified frame index
…source files and to verify an error is thrown if the frame specified is out of bounds
| before do | ||
| @thumb = Paperclip::Thumbnail.new(@file, geometry: "50x50", frame_index: 20, format: :jpg) | ||
| end | ||
|
|
|
|
||
| #List of multi frame formats to check against the source file type | ||
| #this is not an exhaustive list, should be updated to include more formats |
| format: :jpg, | ||
| ) | ||
| end | ||
|
|
| geometry: "50x50", | ||
| frame_index: 20, | ||
| format: :jpg, | ||
| ) |
There was a problem hiding this comment.
@tute can you help me out with what Hound wants here? I have tried a few different options for aligning the ( ), but it keeps reporting errors
|
@tute I think the rest of these issues are just minor things. Hound is reporting some things I don't understand, like it's asking me to use the hash syntax, when I am using the hash syntax. let me know if you want me to address anything else here. |
|
Hound is asking you to use 1.9 hash syntax, e.g. |
|
@bdewater tired eyes, I thought it was talking about the #{ } syntax in strings. Thanks for clarifying. =) |
|
Merged into master. Thank you! :) |
add optional parameter in style options to specify a frame_index to use for generating thumbnails from videos or pdfs. If no value is set, it will default to 0 to maintain default paperclip functionality.