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

Stdin input and --relative-urls option produces wrong urls #3038

Closed
jhnns opened this issue Mar 9, 2017 · 6 comments
Closed

Stdin input and --relative-urls option produces wrong urls #3038

jhnns opened this issue Mar 9, 2017 · 6 comments

Comments

@jhnns
Copy link
Contributor

jhnns commented Mar 9, 2017

When:

  • the content is passed via stdin
  • and the --relative-urls option is used
  • and the imported file is two directories up (../../)

the generated urls are wrong.

Actual output:

.b {
  background-image: url("b/b/test.jpg");
}

Expected output

.b {
  background-image: url("../../b/b/test.jpg");
}

I've created a small test repository that demonstrates the bug. The problem is somewhere along these lines and can be reproduced with less 2.x and less@3.0.0-alpha.1. alpha.2 is throwing an unrelated error.

@seven-phases-max
Copy link
Member

seven-phases-max commented Mar 17, 2017

This is more like an undefined behaviour rather than a bug. By definition, --relative-urls should use a path of the root file as the base, but for stdin input this path is unknown (cwd cannot be used as such since it may be something like lessc -ru < foo/bar/a.less after all).

@jhnns
Copy link
Contributor Author

jhnns commented Mar 20, 2017

cwd cannot be used as such since it may be something like lessc -ru < foo/bar/a.less after all

I would expect it to be cwd. To me, it's the only path that makes sense if you use stdin and --relative-urls.

@seven-phases-max
Copy link
Member

I would expect it to be cwd.

This means guesswork and dirty kludging. While in some workflow cwd is often the path of some file fed via stdin, these things are totally unrelated in fact.

Thinking of it more I begin to believe that for this (stdin + -ru or other rootfilepath related options combo) use-case, the only proper fix is to provide the filename option for lessc too (for now the option is only available via API). Essentially the problem is just the same as in #2342.

@jhnns
Copy link
Contributor Author

jhnns commented Mar 21, 2017

While in some workflow cwd is often the path of some file fed via stdin, these things are totally unrelated in fact.

That's true. It's implicit. It's just that I was assuming it so I thought it might feel natural to others too, but I might be wrong.

Thinking of it more I begin to believe that for this (stdin + -ru or other rootfilepath related options combo) use-case, the only proper fix is to provide the filename option for lessc too (for now the option is only available via API)

That is a good idea. I would try to streamline the node api with the cli as much as possible 👍

@zanona
Copy link

zanona commented Nov 9, 2017

Hey guys, perhaps a --filename option that could be sent when using stdin input would help on this? For example:

cat anotherfile.less | lessc --filename=/path/to/custom/filename.less -

@stale
Copy link

stale bot commented Mar 9, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Mar 9, 2018
@stale stale bot closed this as completed Mar 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants