Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation misuses the -format argument with the convert command #66

Open
Javyre opened this issue Oct 2, 2020 · 7 comments
Open

Comments

@Javyre
Copy link

Javyre commented Oct 2, 2020

Related: #51, #53

The convert command uses the filename extension to know what the output format should be. The filename extensions given to temporary files by waffle is unreliable so the documentation specifies the -format argument. The -format argument to specify file format type is only available in the mogrify command and so this argument has no effect.

The result is that waffle's documentation essentially suggests users use a noop command to convert files and the result is very confusing as the output file extension is right but no conversion has been done.

https://imagemagick.org/script/command-line-processing.php#input
https://imagemagick.org/script/command-line-options.php#format

Environment

  • Elixir version (elixir -v): N/A
  • Waffle version (mix deps): 1.1.3 (N/A)
  • Waffle dependencies when applicable (mix deps): N/A
  • Operating system: N/A

Expected behavior

Uses of the convert command should set an explicit output format like so:

  def transform(:thumb, _) do
    {:convert, 
     fn i, o -> [i, "-thumbnail", "100x100^", "-gravity", "center", "-extent", "100x100", "png:#{o}"],
     :png}
  end

Actual behavior

Uses of convert erroneously use the -format argument to specify output file format. This usage of the -format argument only works in the mogrify command.

  def transform(:thumb, _) do
    {:convert, "-thumbnail 100x100^ -gravity center -extent 100x100 -format png", :png}
  end
@achempion
Copy link
Member

Probably related to #67 as well.

@achempion
Copy link
Member

The filename extensions given to temporary files by waffle is
unreliable so the documentation specifies the -format argument

Temporary file extension will be the same as from original file.

The result is that waffle's documentation essentially suggests users
use a noop command to convert files and the result is very confusing
as the output file extension is right but no conversion has been done.

Could you, please, provide an example?

Do you mean that -format option is unnecessary to perform convert
action using convert utitily?

@Javyre
Copy link
Author

Javyre commented Oct 17, 2020

Could you, please, provide an example?

Here is an example of this being the suggested way in the documentation.

Do you mean that -format option is unnecessary to perform convert
action using convert utitily?

I mean that the -format argument is completely irrelevant to the file format conversion intended. Adding this parameter to the convert command will not get you any closer to actually doing any file format conversion.

This is explained in the imagemagick documentation I linked to: https://imagemagick.org/script/command-line-options.php#format

When used with the mogrify utility, this option converts any image to the image format you specify.

@achempion
Copy link
Member

Here is an example of this being the suggested way in the documentation.

I meant the example for output file extension is right but no conversion has been done.

It sounds like a bug.

@Javyre
Copy link
Author

Javyre commented Nov 4, 2020

I meant the example for output file extension is right but no conversion has been done.

Sorry I think that sentence wasn't very clear. What I meant by that was that the file extension specified in the argument :png and in the -format parameter would lead the user to think that they're doing everything right, but no conversion happens.

Temporary file extension will be the same as from original file.

That's fine, but until it's specified behavior in the documentation of waffle, I'd see that as a pretty unreliable implementation detail. Either way the -format parameter would remain misleading to users.

@achempion
Copy link
Member

So, basically we should remove the -format option from tests/docs to not confuse users because this option wouldn't be applied.

@Javyre
Copy link
Author

Javyre commented Nov 5, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants