vseventer / sharp-cli

CLI for sharp.
MIT License
186 stars 20 forks source link

`{dir}` is incorrect when using with url-template -o parameter #9

Closed thom4parisot closed 6 years ago

thom4parisot commented 6 years ago

Hello!

Thanks for this tool, it works beautifully and is probably the best in town :-)

I discovered url-template but the {dir} directive does not correspond to what I expected. Ref to require('path').parse.

For example, this is a command I used and did not work as expected:

$ sharp -i content/playstation/images/boite-de-console-playstation-europeen.jpg -o '{dir}/{name}.sq{ext}' resize 170 170

vips__file_open_write: unable to open file "/Users/oncletom/workspace/emunova/emunova.net/%2FUsers%2Foncletom%2Fworkspace%2Femunova%2Femunova.net%2Fcontent%2Fplaystation%2Fimages/boite-de-console-playstation-europeen.sq.jpg" for writing
unix error: No such file or directory

I was expecting {dir} to be equal to either content/playstation/images or /Users/oncletom/workspace/emunova/emunova.net/content/playstation/images.

{dir} looks like an encoded value of some sort, which is prefixed by $(pwd).

Am I using it wrong?

Thanks for your help :-)

thom4parisot commented 6 years ago

I have inspected the problem a bit further.

url-template

It seems that {dir} is URL encoded by url-template, maybe because / is considered as an operator (don't know what it means):

https://github.com/bramstein/url-template/blob/c71192027d8056994be5fcecdb88f3db919f45da/lib/url-template.js#L152

const urlTemplate = require('url-template')
const template = urlTemplate.parse('{dir}/{name}.sq{ext}')

template.expand({ dir: 'test', name: 'a', ext: '.jpg' })
// outputs "test/a.sq.jpg"

template.expand({ dir: 'test/test', name: 'a', ext: '.jpg' })
// outputs "test%2F/a.sq.jpg"

absolute path + url-template

The problem lives here:

https://github.com/vseventer/sharp-cli/blob/87b9340c9197aed069750a23c23c4938677cd3fd/lib/convert.js#L52-L53

If {dir} leads, or if output equals /mnt/otherdrive/{file}{ext}, output will be absolute-ised prior to its expansion.

Maybe it should be absolute-ised later, if not already absolute (cf. path.isAbsolute) like here, to benefit from url-template:

https://github.com/vseventer/sharp-cli/blob/87b9340c9197aed069750a23c23c4938677cd3fd/lib/convert.js#L59

Maybe some additional modifiers like {cwd} and {relative_path} (between file and cwd) would help tuning the dest path too.

What do you think?

vseventer commented 6 years ago

Thank you for looking into this - I think I'll have to rethink using url-template, as the slashes will always be encoded. I see your PR solves this, but ideally I'd like to avoid using decodeURIComponent.

vseventer commented 6 years ago

@oncletom The format you are looking for (in your opening post) is {+dir}/{name}.sq{ext}. The plus symbol indicates no characters will be encoded, per RFC 6570.

thom4parisot commented 6 years ago

Oh bummer! Thanks for the heads upb and for the finding; I totally missed it from the syntax!

Than you so much :-)