allow loading extensions when a trigger is loaded from below the parent's load path#49701
Merged
KristofferC merged 4 commits intomasterfrom May 12, 2023
Merged
allow loading extensions when a trigger is loaded from below the parent's load path#49701KristofferC merged 4 commits intomasterfrom
KristofferC merged 4 commits intomasterfrom
Conversation
Member
Author
|
I went ahead with the change to allow extensions getting loaded for the active project. So as an explicit example, if you have PGFPlotsX as your current active project, and then load Colors from the global environment, the extension for Colors in PGFPlotsX will now load. I think this is what people naturally expected anyway. |
Member
|
I agree that sounds much more logically consistent. |
staticfloat
approved these changes
May 12, 2023
Member
staticfloat
left a comment
There was a problem hiding this comment.
Works for me on my workload!
This was referenced May 15, 2023
This was referenced Jun 1, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@topolarity has convinced me that this makes sense.
As it is right now, an extension gets loaded only if the trigger is above (or at the same place) as the parent in the load path.
This is kind of a symmetry break because, from the extension's point of view, there isn't really any difference between the parent and one of the triggers. Both of these are just dependencies.
The extension could be shifted from the parent to the trigger without any change of functionality.
So then why does code loading differ between these two cases?
With this PR, the extension will load no matter the relationship of load paths between the parent and triggers.
This also avoids the complication in #49681.
cc @vtjnash, @topolarity
Also, a TODO is that extensions should probably be loadable if the parent is the active project (which would need to add some code to look up extensions in Project.toml files, right now it only looks inManifest.toml). Fortunately, I envisioned maybe having to do this so I saved some code I ripped out earlier that implements this: https://gist.github.com/KristofferC/e5c9e80db0f056ab1f72714c46477a02.^Done
Fixes #49656