Provide a twig extension for file_create_url

Perhaps just file_url()?

i.e.:

<img src="{{ file_url('core/misc/arrow-asc.png') }}">

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -53,6 +53,8 @@ public function getFunctions() {
+      new \Twig_SimpleFunction('file_rl', 'file_create_url')

Typo /file_rl/file_url/

And I like the short one too, and would favour ditching the longer one to keep the namespace clean, though love to hear others take on that? Discussed a bit here already #2168231-7: Twig Functions needed in templates

Fixed TYPO.

@dawehner do we need tests to confirm that the file_create_url() works?
We have a number of tests that seem to do this like Drupal\file\Tests\DownloadTest::testPublicFileTransfer checks that it works correctly.

Twig is just passing the function to the template which it has tests for itself and we have tests for how it works, so I'm inclined to think we don't need tests.

+++ b/core/lib/Drupal/Core/Template/TwigExtension.php
@@ -53,6 +53,8 @@ public function getFunctions() {
+      new \Twig_SimpleFunction('file_create_url', 'file_create_url'),
+      new \Twig_SimpleFunction('file_url', 'file_create_url')

Not sure we should go with two, and we need a comma at the end of the second line regardless. Can we settle on file_url?

We also do tests with link generation so it's good to have tests for this.

Not sure we should go with two, and we need a comma at the end of the second line regardless. Can we settle on file_url?

It would make sense to have single function rather than having aliases (would be confusing?). In the other hand we could consider renaming whole file_create_url() function in the first place which would be then yet another topic.

I apologize for the drive-by Dreditor…

+++ b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.file_url.html.twig
@@ -0,0 +1,2 @@
+<div>file_create_url: {{ file_create_url('core/assets/vendor/jquery/jquery.js') }}</div>
+<div>file_url: {{ file_url('core/assets/vendor/jquery/jquery.js') }}</div>
\ No newline at end of file

Thanks @iMiksu! Please add a newline at EOF for this test Twig template. Otherwise looks good at a glance and is failing/passing appropriately.

Fixed some documentation in the tests. Saw that earlier and wanted to s/twig/Twig. Also fixed the missing newline.

@iMiksu maybe we just leave the name the same (less docs to change;) ). I'd rather have one function than two because it will just split the people who are using it in contrib and core and just confuse.

I'd rather have it consistent yet annoyingly named and longer than necessary than have two.

We can't make those names changes easily in beta, so I'd rather look at that for a 8.1 or more likely 9.x change, but we can open that issue now and postpone;)

I agree with using one function. My initial opinion was to use file_create_url as it maps directly, making it much easier to track down. We also have precedence using that function name. I do like that file_url is shorter and friendlier, which is a plus in Twig templates.

If using the shorter name is a change that may not be possible in beta (Though I'm not sure why. Wouldn't this technically be an api addition?) then take the path of least resistance and use the longer one.

What docs need to be changed? Is it just the theming guide docs, or do we need to make a bunch of comment/api.drupal.org changes?

Changed the latest patch use only file_create_url

Accidental change in core/modules/taxonomy/src/Tests/ThemeTest.php file?

--- a/core/modules/taxonomy/src/Tests/ThemeTest.php
+++ b/core/modules/taxonomy/src/Tests/ThemeTest.php
@@ -44,7 +44,7 @@ function testTaxonomyTermThemes() {
     // Viewing a taxonomy term should use the default theme.
     $term = $this->createTerm($vocabulary);
     $this->drupalGet('taxonomy/term/' . $term->id());
-    $this->assertRaw('bartik/css/style.css', t("The default theme's CSS appears on the page for viewing a taxonomy term."));
+    $this->assertRaw('bartik/css/base/elements.css', t("The default theme's CSS appears on the page for viewing a taxonomy term."));

Dont know how that ended up there :P

Wtf I messed up the patch and even RTBC'd by a mistake. Im such newbie!

@lauriii and @iMiksu want to give @davidhernandez's suggestion a try and aim for file_url and fallback to file_create_url if there is too much strife? It would be an API addition if we just do the twig function change.

We discussed this on yesterdays Twig call and agreed that we want to have the shorter one aka file_url. We have hook_file_url_alter so its aligned with that. It's also a lot more easier to remember and makes more sense.

We don't want to have both to make it as clear as possible. If we would use both we would end up having both versions in contribs and core and people might get confused of whats the difference. Its also not good in terms of consistency.

We don't have verbs in other filters neither so it would be also inconsistent with that.

@lauriii thanks for rolling that up.

Here is the reference we were talking about @see https://api.drupal.org/api/drupal/modules!system!system.api.php/function...

+++ b/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php
@@ -52,9 +52,9 @@
--- b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.file_create_url.html.twig

--- b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.file_create_url.html.twig
+++ /dev/null

@@ -1 +0,0 @@
-<div>file_create_url: {{ file_create_url('core/assets/vendor/jquery/jquery.js') }}</div>

Is it just missing this file_url.html.twig?

Omg! Im such newbie

You are mixing your patches with other patches. You have the breadcrumb stuff in here.

No worries man, that is the most common thing on re-rolls to miss new files it seems, we all do it. For the mixing of patches, if you don't already, create a branch per issue. That usually mitigates that for me, before I used to just use git diff --staged but that was making too many mixing issues;) Thought branch per issue doesn't solve that issue always... I still do that when I get lazy and forget to create a new branch for a "quick fix"

Sorry for messing around a bit :D

Great let's get this in play. Thank you!

1. This has tests.
2. It's an API addition for Twig to deal with assets.
3. Matches the alter hook that affects it (hook_file_url_alter)

Change recorded added: https://www.drupal.org/node/2396439

+++ b/core/modules/system/src/Tests/Theme/EngineTwigTest.php
@@ -83,4 +83,13 @@ public function testTwigLinkGenerator() {
+    $filepath = file_create_url('core/assets/vendor/jquery/jquery.js');

+++ b/core/modules/system/tests/modules/twig_theme_test/templates/twig_theme_test.file_url.html.twig
@@ -0,0 +1 @@
+<div>file_url: {{ file_url('core/assets/vendor/jquery/jquery.js') }}</div>

This is a bad test file. How about creating a file in the test. Then the test has no hidden dependencies?

Are you saying to dynamically create a file, or add/keep one in the test's directory to use for the test?

#44 either is fine by me. Just linking to an asset that might change is a bad idea.

Will this do the trick?

@dawehner added this to the issue summary:

Reason we shouldn't put both file_url() and file_create_url() into twig is because it would split the function usage and make consistency hard to accomplish as some would use the old version and some would use the shorter new one.

That work?

Also it would be confusing for a while, why there are two functions doing the same thing... specially if someone decided to use the alternate on a project.

Can we rename the php one? :)

Yeah we are playing with what we have here... don't want an API change, but want a better name. The name has a side benefit of matching better to the hook that acts on it. So we have that improvement... Would be nice to do that PHP function name change for D9.

Couldn't we essentially alias it and mark the longer one deprecated to be removed later? ...but I digress.

Nitpicks that can be fixed on commit:

1. +++ b/core/modules/system/src/Tests/Theme/EngineTwigTest.php
   @@ -83,4 +83,13 @@ public function testTwigLinkGenerator() {
   +   * Tests the file url Twig functions.
   

   s/file url/file_url/

2. +++ b/core/modules/system/tests/modules/twig_theme_test/src/TwigThemeTestController.php
   @@ -49,4 +49,13 @@ public function linkGeneratorRender() {
   +   * Renders for testing file_create_url functions in a Twig template.
   

   s/file_create_url/file_url/

Committed 6df284e and pushed to 8.0.x. Thanks!

Thanks for adding the beta evaluation for to the issue summary.

#58 was not fixed on commit. Of course that's no disaster, but if those who worked on this patch would roll a patch to fix it in a follow-up issue, ping me for review!

Posted follow up. #2405823: Comment fix in Twig tests for file_url