Unused variable url_options in file_token

andrew.farquharson → made their first commit to this issue’s fork.

Hi kim.pepper and larowlan,
I have searched core codebase recursively for the line,

$url_options = ['absolute' => TRUE];

The line is redundant and the variable is not used in the file module. So I believe it can be safely removed.

LGTM, but unsure whether the variable should be used.

Changing to needs review.

Left a comment.

Hey @smustgrave,

Completely missed that other line, apologies! I've created a new MR which removes both lines. Thanks!

This is a small change but just FYI if there's already an MR should update that one vs opening a new one.

We should look through git history to find out when this line was introduced, and determine if it was later missed in a refactoring (and should be removed) or if it's a bug (and should be kept and used somewhere).

@smustgrave and @longwave I recommend a recursive search through the core folder. Search on '$url_options['language']' and you get 7 occurrences in 7 different files including file.module. Search on '$url_options = ['absolute' => TRUE];' and you get exactly 7 occurrences in the same 7 files. Both these lines are entirely redundant I believe and should be deleted from all 7 of the files (14 lines in total).

Perhaps create related tickets for the other core modules in which the lines appear.

Hi,

To remove unneeded variables from the file.module, I attempted this patch.

Thank you for the interest but the patch is just a copy for the MR so removing credit for duplicate work.

Believe what longwave was asking was to review the git history for this snippet of code

I have looked through the gitlab history of the file.module file.

The 2 lines of code that are now the subject of this issue seem to have first appeared in
commit file.module

The commit with SHA 988abc27f8b2f1a05e42932a39954dec4f18c09a was by Nathaniel Catchpole on 21 July 2013. In the description he refers to issue 2045189 →

So it seems that the source of the 2 $url_options lines is patch by jlindsey15 →

Just to make things easier, here are the files that have the mention of the '$url_options['language']' line:

core/lib/Drupal/Core/Utility/token.api.php
core/modules/comment/comment.tokens.inc
core/modules/file/filemodule.php
core/modules/node/nodetokens.inc
core/modules/system/system.tokens.inc
core/modules/user/user.tokens.inc
core/modules/views/views.tokens.inc

I did some digging around for token.api.php and it seems like the file was created to shift the token related hooks from system.api.php. This was done on the 17th of April 2015 by Jess via issue #2470976 and commit SHA fd66ee2a9eb25ad954d5f417103127b8af280a4a

I looked to see when the code was added in the original system.api.php and found out that the earliest addition of this code was on the 8th of January 2010 by Dries Buytaert via issue #673462 and commit SHA 4b23d00e602a1be6b7d84549bbac2888a2453ee6. As far as I know, this is where the code for hook_tokens() was added.

oily → changed the visibility of the branch 3368846-unused-variable-urloptions to hidden.

I have hidden stale branch.

oily → closed merge request !4245

oily → reopened merge request !4245

The MR is resolved. Change to 'Needs Review'.

oily → changed the visibility of the branch 11.x to hidden.

Closing this as seems already fixed.