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

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

Closed
oncletom opened this Issue Nov 9, 2017 · 4 comments

Comments

Projects
None yet
2 participants
@oncletom

oncletom commented Nov 9, 2017

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 :-)

@oncletom

This comment has been minimized.

Show comment
Hide comment
@oncletom

oncletom Nov 10, 2017

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:

sharp-cli/lib/convert.js

Lines 52 to 53 in 87b9340

output = path.resolve(output) // Absolute path.
const template = urlTemplate.parse(output)

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:

let dest = template.expand(path.parse(src))

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

What do you think?

oncletom commented Nov 10, 2017

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:

sharp-cli/lib/convert.js

Lines 52 to 53 in 87b9340

output = path.resolve(output) // Absolute path.
const template = urlTemplate.parse(output)

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:

let dest = template.expand(path.parse(src))

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

This comment has been minimized.

Show comment
Hide comment
@vseventer

vseventer Jan 3, 2018

Owner

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.

Owner

vseventer commented Jan 3, 2018

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 vseventer closed this in 9ef1c73 Jan 5, 2018

@vseventer

This comment has been minimized.

Show comment
Hide comment
@vseventer

vseventer Jan 5, 2018

Owner

@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.

Owner

vseventer commented Jan 5, 2018

@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.

@oncletom

This comment has been minimized.

Show comment
Hide comment
@oncletom

oncletom Jan 5, 2018

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

Than you so much :-)

oncletom commented Jan 5, 2018

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

Than you so much :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment