Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force focus on MAIN when loading file #26

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

snoato
Copy link
Contributor

@snoato snoato commented Dec 16, 2024

If files declare a module, the focus is switched to this module. That means that rules, functions, etc. of subsequently loaded files are also loaded into that module. Therefore we switch the focus in the file load plugin to MAIN before loading a new file to prevent this unwanted side effect.

@snoato snoato requested a review from TarikViehmann December 16, 2024 14:47
@@ -12,6 +12,7 @@
; See the License for the specific language governing permissions and
; limitations under the License.

(defmodule MAIN (export deftemplate time))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this necessary?

Shouldn't this be up to the user (e.g., to re-define the main module once and then exporting whatever is necessary)?

I have not worked with modules yet, but it seems if this export here is necessary, then this needs to be a policy across all plugins.

@@ -70,6 +70,8 @@ bool FileLoadPlugin::clips_env_init(LockSharedPtr<clips::Environment> &env) {
context->env_name_.c_str());
for (const auto &f : init_files_) {

clips::Defmodule *main_module = clips::FindDefmodule(env.get_obj().get(), "MAIN");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants