While the very concept of fallbacks are undocumented (hurray...) reading the code I was able to figure out what they do. I recommend the following patch so that the next dude reading this code has an easier time.

CommentFileSizeAuthor
fallbacks_not_namespaced.patch528 byteschx
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

Chx can you explain some more why the change is needed. Or why it is an improvement. Changing this line doesn't tell me what was wrong with the previous one. Thanks!

chx’s picture

The key is not used by Symfony and it is very confusing to have the key when you don't need it. Nonfallbacks use their key in the path you would then expect that this key means something and it doesnt. Better to remove.

RobLoach’s picture

I'm not sure this is a good idea. This will make it so that any class that's loaded will be checked in /core/lib after the initial namespaces are checked. It's better to be verbose with the namespaces. #1658720: Use ClassLoader instead of UniversalClassLoader

RobLoach’s picture

Status: Needs review » Postponed

Let's wait for #1424924: Use Composer for updating Symfony components (without removing Symfony code from repo) to get in too, since that's already RTBC and Angie wanted to keep the threshold down.

chx’s picture

@RobLoach you have no idea what are you talking about I am afraid. The key in fallback is not used at all. There's nothing verbose vs terse. It's astonishing this patch haven't made it in, of course now it's obsolete but it should have been a slam dunk except noone has the slightest idea what happens because this code calls Symfony which I did read and noone else in this issue did. This is what we get for code reuse and it worries me to no end.

RobLoach’s picture

I don't mean verbose, I mean explicit. A project should register its own namespaces. If you just register a path without the namespace, that means it'll check that path last if no other namespace check is met. I understand that both accomplish the same thing, but it's good to explicitly state where your namespaces live.

Drupal\Core lives at core/lib
Drupal\Components also lives at core/lib

Register those namespaces to their correct locations. If we don't, then "MyRandomNamespace\ChxClass" will be assumed to live at core/lib/MyRandomNamespace/ChxClass.php since "MyRandomNamespace" is not registered.

What we need are patches and benchmarks.

mgifford’s picture

No reason to postpone this anymore.

chx’s picture

Status: Needs work » Closed (fixed)

Neither do we use that classloader any more.