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

Update filter.php for backward compatibility #55

Merged
merged 1 commit into from
Oct 18, 2024

Conversation

michael-milette
Copy link
Contributor

This makes the plugin backward compatible with versions of Moodle up to 4.5.

@michael-milette
Copy link
Contributor Author

michael-milette commented Oct 17, 2024

Addresses a compatibility issue with versions of Moodle up to 4.4 as described on the following pages:

https://moodle.org/mod/forum/discuss.php?d=462953

and

https://moodle.org/mod/forum/discuss.php?d=462961

Best regards,

Michael Milette

@iarenaza
Copy link
Owner

Humm, we had that before, and I removed it. And as I commented in issue #54, the code works just fine in Moodle 4.1.13 (the version one of the error reporters is using) and 4.1.14+ (the one tested by the CI matrix when I last pushed the changes). As long as your PHP or Moodle caches (I don't know which one caches the classes paths) is not stale.

The problem with your proposed PR is that, as stated in commit 35befeb, "moodle-plugin-ci validate" insists on finding class filter_multilang2 in file filter.php. But the class aliasing construct is not recognized by the PHP parser used by moodle-plugin-ci (and adding that support is a rather major effort, believe me, I tried and failed miserably 😄).

Which means that, if we apply your PR, we cannot use moodle-plugin-ci validate in the CI pipeline (or we have to ignore its error output, as the initial PR from @lucaboesch did). Which is a major drawback IMHO.

I'm not completely sure if adding a use \filter_multilang2\text_filter at the top of the file would solve things in this case, to be honest.

@iarenaza
Copy link
Owner

Given that hopefully moodle-plugin-ci will be fixed soon with moodlehq/moodle-plugin-ci#326, I'm going to apply this PR.

@iarenaza iarenaza merged commit ee131b4 into iarenaza:master Oct 18, 2024
0 of 28 checks passed
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