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

'auto' support in fetch and fetchFormat #7

Merged
merged 1 commit into from
May 15, 2021
Merged

Conversation

apalumbo
Copy link
Contributor

In version 1.x 'auto' was supported as fetch format, this PR add it also to version 2.x

@apalumbo apalumbo mentioned this pull request Feb 20, 2021
@yoadsn
Copy link

yoadsn commented Apr 23, 2021

Hi @apalumbo I am also interested in helping to maintain this.
I think that fetch does not accept auto according to spec - only the fetchFormat.
Right?
See here compare with here

@apalumbo
Copy link
Contributor Author

Not sure, I mean at the end of the day it is the same parameter f_auto for automatic or f_jpg for jpg
Am I missing something?
@yoadsn

@marnusw marnusw merged commit e9b3d98 into marnusw:master May 15, 2021
@marnusw
Copy link
Owner

marnusw commented May 15, 2021

Thanks @apalumbo. It looks like this was just a types issue because it was supported in the code before. As for fetchFormat vs. format as I recall I couldn't find fetchFormat in the current docs anymore, and the two options have always basically done the same thing so it would make sense that they have silently deprecated one. In this lib it's also an alias of format, both generating f_<value>, so they both do support auto.

@yoadsn
Copy link

yoadsn commented May 15, 2021

It translates into the same f_ directive indeed. But, the actual spec indicates only "fetchFormat" accepts auto.
I guess it's a matter of supporting what the code does - and indeed in the code they are used interchangeably (i.e. if not "fetchFormat", take from "format")

@yoadsn
Copy link

yoadsn commented May 15, 2021

@marnusw If you follow the links I have posted above (the docs) and choose "JS" flavor - you will see it uses "fetchFormat" for the "auto" value and not "format".

@marnusw
Copy link
Owner

marnusw commented May 15, 2021

O, now I see, you're right @yoadsn. Thanks. Still, since it makes no difference in terms of the output I think it's fine for the types to support both in this case.

It's also possible those docs are out of date, because I see the (I assume) newer PHPv2 and Kotlin examples do use a variant of format: auto.

@marnusw
Copy link
Owner

marnusw commented May 15, 2021

v2.0.1 is available.

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

Successfully merging this pull request may close these issues.

None yet

3 participants