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

Passing filepath to the Pug compiler to allow using relative paths #9

Merged
merged 1 commit into from
Mar 27, 2017

Conversation

vitalybaev
Copy link
Contributor

Using leading slash to using Jade's extend breaks code completion in IDE's.
jade-phpstorm
But we could simply pass filepath to the Pug compiler, and get opportunity to write relative paths.

@weotch weotch merged commit 3cbafe9 into BKWLD:master Mar 27, 2017
@vitalybaev
Copy link
Contributor Author

@weotch Thanks. Could you also tag changes?
Cause the last tag is dated 29 Jul 2016 and I have to using "dev-master" version in composer

@vitalybaev vitalybaev deleted the pass-filepath-to-pug-compiler branch March 27, 2017 17:35
@weotch
Copy link
Member

weotch commented Mar 27, 2017

Was just looking into this for you and I noticed this commit:

874deb5

  1. It is no longer true that you need a leading slash, right?
  2. Can you test your implementation of your PR and see if the basedir is still required?

@vitalybaev
Copy link
Contributor Author

vitalybaev commented Mar 27, 2017

@weotch So for now:

  1. Yep, you could use any relative paths in Jade/Pug templates as usual
  2. basedir don't required if you are using relative paths, and required if you are using absolute paths (using leading slash):
The 'basedir' option need to be set to use absolute path like

@kylekatarnls
Copy link
Collaborator

In pug-symfony, I set the basedir and pass the file path and resolve paths from each possible path (bundles in Symfony), so both relative and absolute always works. There is no reason it would not be possible to do the same. Your both implementations seems to solve all problem with that. But as far as I understand, there is no longer any laravel-pug specificity for extending layouts. It should works as in pug and so you could just add a reference to pug-js documentation.

@vitalybaev
Copy link
Contributor Author

@kylekatarnls but my PR doing exactly the same. Before only basedir was specified, path was not passed.

weotch added a commit that referenced this pull request Mar 27, 2017
@weotch
Copy link
Member

weotch commented Mar 27, 2017

@vitalybaev Does 1dac714 look accurate to you?

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 27, 2017

@vitalybaev Yes the coding work seems done to me and well done.

@weotch Better but still a bit confusing:

you must specify a `basedir` in the config file (it defaults to `resources/views`).

No you must not. As a user the basedir is set for you and the resources/views default is relevant for most users.

You don't need to change it, best practice would even be to keep it as a standard. But you can say something like:

You can always customize the views root directory with `basedir` option if you need absolute paths to be resolved from another directory.

@kylekatarnls
Copy link
Collaborator

kylekatarnls commented Mar 27, 2017

This is what I would say to explain: #10 feel free to take or adapt.

@vitalybaev
Copy link
Contributor Author

@weotch Yes, looks good!

@vitalybaev
Copy link
Contributor Author

@weotch @kylekatarnls Tell me guys one question: should this call be also pass $path?

https://github.com/BKWLD/laravel-pug/blob/master/src/PugBladeCompiler.php#L44

@weotch
Copy link
Member

weotch commented Mar 27, 2017

It makes sense to me that it should. I'll do that now.

weotch added a commit that referenced this pull request Mar 27, 2017
@vitalybaev
Copy link
Contributor Author

@weotch Good job! Thanks for the package. The only thing is publishing release for using it with composer semver.

@weotch
Copy link
Member

weotch commented Mar 27, 2017

Cool! Released now as 1.2.0

@vitalybaev
Copy link
Contributor Author

@weotch @kylekatarnls I took a look with fresh mind and realised that I misunderstood you yeasterday: I've completely removed basedir from vendor config. That is the reason, I've got error. 😄
Thank you guys for help 👍

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