Civi::paths() - Allow bare variable lookup via getUrl/getPath
Overview
The Civi::paths()->getUrl(...)
and Civi::paths()->getPath(...)
have a quirky interpretation of bare variable expressions (eg getPath('[civicrm.packages]')
).
Example use-case
# (A) With a file/folder/subpath
cv ev "echo Civi::paths()->getUrl('[civicrm.packages]/foo/bar');"
# (B) No slash, no dot
cv ev "echo Civi::paths()->getUrl('[civicrm.packages]');"
# (C) With a slash
cv ev "echo Civi::paths()->getUrl('[civicrm.packages]/');"
# (D) With a slash and dot
cv ev "echo Civi::paths()->getUrl('[civicrm.packages]/.');"
Current behavior
In scenarios (A), (C), and (D), the value of [civicrm.packages]
is substituted. But in situation (B), it is treated as a literal file-name and given the default prefix.
$ cv ev "echo Civi::paths()->getUrl('[civicrm.packages]');"
http://site/[civicrm.packages]
Proposed behavior
All examples -- including (B) -- should do substitution.
The Civi\Core\PathsTest
should be expanded to check scenario (B).
Comments
Historically, when getPath()
/getUrl()
were first drafted to support expressions like [civicrm.files]/persist/contribute
, the primary concerns were (1) backward compatibility for older absolute+relative expressions (without any [foo]
expressions) and (2) new use-cases like (A). The [foo]
notation was conceived as an optional prefix (with an implied default) - and not as a straight-up variable. And if you just wanted a raw variable, I'd've imagined it'd be faster to call getVariable()
(bypass the string-munging with getPath()
etc).
OTOH, if you learned of this API by skimming docblocks or examples (without that historical context), then it'd be natural to assume that (B) would work, and it really is a more approachable interface that way.
Strictly speaking, it is a change in the contract for an obscure edge-case: if you had a file named /var/www/sites/default/files/civicrm/[foo]
, and if you requested getPath('[foo]')
, then it currently resolves to the file. With this change, it would interpret [foo]
as a variable - the variable is probably undefined, which leads to an exception. You'd have to rewrite the call as getPath('[civicrm.files]/[foo]')
. That's an exceedingly marginal edge-case, and I don't really think it's worth preserving.