Rename images when converting formats

[jcarlson]

<< new issue based on #403 >>

This should help with situations where a file has multiple versions, and at least one of the versions is of a different format than the master version. This scenario is documented in issue #378 as well as in issue #284 (and possibly others).

Simply pass :rename => true to the existing convert helper method, and the file will be renamed as well as converted. Existing functionality is preserved, i.e., the default is to not rename the image.

When using Fog as the storage provider, you can utilize process :set_content_type to detect the new MIME type and have that relayed to the cloud storage provider.

[jcarlson]

FYI, without this patch, if you were to call any processing commands after a :convert call, the file would be converted back to it's original format. For example

# assume we are working on a 'pretty_flower.jpg' image
process :convert => :png  #after convert, file retains .jpg extension
process :resize_to_fit => [250,250] #rmagick converts back to a JPG when it writes out the file, due to .jpg extension

There are, of course, other ways to work around this limitation, such as writing a custom processor that just renames the file, but was curious as to anyone else's thoughts.

[bensie]

This looks good, though I don't like to encourage the use of methods like convert("png", false). Would you mind reworking to have the convert method take an options hash as its second argument?

Also, might you be up for tackling a test here?
https://github.com/jnicklas/carrierwave/blob/master/spec/processing/rmagick_spec.rb#L23

[bensie]

After testing this locally, I see it breaks a few specs because of undefined local variable or method 'current_path'

Feel free to submit a new pull request that fixes this -- and make sure you run the specs and add any relevant tests for the changes you make! Thanks!

[jcarlson]

Yes, I will make the mentioned changes...

[kuraga]

+1 for this feature! But I think rename option should be true by default!

[MikeMcQuaid]

Did this feature ever make it into master?

[mhuggins]

+1, is this available in master?

[kevinwmerritt]

I could use this. Set_content_type does not work after convert => 'jpg'

[ThorKhan]

+1

[cgat]

+1

[shoutsid]

+1 this should be in master. Makes it so much easier when dealing with different file types from the same upload.

[thiagofm]

I would LOVE to see somebody fix what's pending here and submit a pull request, anyone? ❤️

[bogn]

This is a severe issue because as stated already, it's totally hidden and undocumented.

We had convert => 'jpg' as first processor and were relying on the final result to be an actual jpg.

Because uploaded pngs were ultimately not converted to jpgs, those images just caused hours of debugging on IE 8. Because of the header X-Content-Type-Options:nosniff the browser was not allowed to sniff the actual content type (see here why) and thus tried to interpret a png file as a jpg file which it could not do and that way the images didn't appear in that browser.

The issue is 'convert' not being effective when further processors follow. This is completely hidden and the advise from the wiki to override the filename method means the resulting images will always have the desired extension, even if the actual files have a different format. This makes it even less likely this issue is discovered early on.

This should be mentioned in the Readme.md and on the known issues page.

[nashby]

Guys, I create a new pull request #1446. I think it should be default behaviour and we don't need that rename option. What do you think?

[bensie]

#1446 was just merged, thanks @nashby!!

[miguelpeniche]

+1