🇮🇪Ireland @lostcarpark

lostcarpark → created an issue.

This issue seems to be resolved by 🐛 Fix PHP 8.4 implicit nullable deprecation Needs review , so can probably be closed as a duplicate.

I've tested the patch and it's resolved the deprecation warnings for me.

I get the following when running drush updb on a site with pathauto enabled:

PHP Deprecated: Drupal\pathauto\Commands\PathautoCommands::generateAliases(): Implicitly marking parameter $types as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/Commands/PathautoCommands.php on line 89

PHP Deprecated: Drupal\pathauto\Commands\PathautoCommands::deleteAliases(): Implicitly marking parameter $types as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/Commands/PathautoCommands.php on line 135

PHP Deprecated: Drupal\pathauto\AliasStorageHelper::__construct(): Implicitly marking parameter $entity_type_manager as nullable is deprecated, the explicit nullable type must be used instead in /web/modules/contrib/pathauto/src/AliasStorageHelper.php on line 79

I have added a before script to .gitlab-ci.yml to temporarily remove [Hook] and [LegacyHook] attributes.

I'm happy with this, and tests are green so happy to go RTBC.

This could be the same issue as 🐛 SID 2776211 contains div element Needs review , which failed if a "Rich text" text format wasn't configured.

It was fixed by moving the header out of the view, and into the Controller.

If that was the issue, this could probably be closed, but I agree making "Views" a dependency is a good idea.

lostcarpark → created an issue.

lostcarpark → created an issue.

lostcarpark → created an issue.

Added attendees to issue credit using new system.

Duplicate of 📌 Mentor Meeting - 24 September 2025 Active

I think general dependency injection would be next to impossible to do with rector, but converting t() to $this->t() seems like something that could be fairly easily handled by inserting the use statements, and replacing the existing t() calls.

The rector script sounds a great approach to get an initial cut of the object oriented hooks.

However, I think dependency injection is one of the biggest benefits of switching to OO hooks, so I think it's important to treat the script as the first step, and not consider it "job done" when the script has run.

I really look forward to seeing how the script works, and I think it could streamline the conversion process.

I was there...

Hi @vitaliyb98,
I don't think there's any explicit testing required.
Certainly no need to test it won't install against Drupal 9, since all that would test is the Drupal 9 module installer.
In my opinion, since the changes are not to any program code, it is sufficient to eyeball the change, confirm it makes sense, and verify the tests are passing in the pipelines.

Perfect, thanks.

Looking at it now, I'm not sure why the test modules aren't failing with the same error.

Confirm pipeline is now passing, and new "Upgrade status" task is reporting no issues.

Moving to RTBC.

Confirmed all tests are passing.

Moving to Needs Review.

Edit title to remove question

Scheduled Publish 4.1.0 has been released, and should work with Drupal 9, though it hasn't been explicitly tested against it. If there are any security or bugfixes against the 4.1 branch, they will continue to support D9.

However, new features will be developed against the 4.2 branch. In particular, 📌 Convert procedural hooks to object oriented hooks Active is in progress to convert to object oriented hooks. As this feature requires autowired services, it is not possible to implement keeping support for Drupal 9. Therefore, this change is to remove D9 support from the 4.2 branch.

Merge complete.

I appreciate the additional test coverage.

Thank you for your work on this.

I've carried out some manual testing, and merged in previous changes.

Happy to merge in.

I created a test module containing a new plugin with an ID of testflavor, and a label of "Markdown Easy Test Flavor".

I've added a test that tries creating a text format using the plugin, checking that the new flavor exists in the drop-down, and that the correct tags are warned about.

The expected tags are the "standard" list plus <article>.

This looks almost perfect.

Upgrade Status is giving a warning about core_version_requirement in the test theme. I know that line can be omitted in test modules, so I suspect it could be removed from the test theme too. I would try deleting the line and see if the tests pass.

I realised that findMissingTags in MarkdownEasyUtility hard coded the list of tags for each flavor, and also raised an exception if an unknown flavor was used.

So it seemed to make sense to add the expected tags to the flavor attributes.

I though we should keep the "tag sets" so we don't have to define the full list of tags in every plugin, which would require a lot of repetition.

So I've added two new plugin attributes:

• tagSets is an array of tag sets, with the allowed values "standard", "github", and "markdownsmorgasbord".
• additionalTags is an array of individual tags, with the tag name as the array key, and an array of attributes as the value.

This required the MarkdownEasyPluginManager to be injected into the MarkdownEasyUtility service.

This in turn required the MarkdownUtilityTest unit test to be updated with a mocked plugin manager service.

I'm tempted to add a test class that implements a minimal plugin, which should help catch people accidentally adding hard coded stuff in future.

This should be a fairly easy change for someone aiming to build up their contribution experience.

Here's a brief summary of what's needed:

1. Create a Hook directory under /src.
2. Create a new PHP class in the /src/Hook directory. As all the current hooks relate to altering forms, I'd suggest calling the class FormAlterHooks.
3. Create a __construct() constructor to set up dependency injections. It should be able to accept the services: config.factory, string_translation, markdown_easy.utility, messenger.
4. Copy the hook functions from markdown_easy.module into the hook class. As a general convention, the "markdown_easy" prefix should be dropped from the prefixesshould be dropped, and the function names should be converted from snake case to camel case.
5. Add the #[Hook] attributes to the functions. For example the current markdown_easy_form_filter_format_add_form_alter function might become formFilterFormatAddFormAlter, and its attribute should be #[Hook('form_filter_format_add_form_alter')].
6. Functions starting with a prefix are utility functions, and can be converted to private methods of the class.
7. Any \Drupal calls should be replaced with the dependencies injected in the constructor.
8. Add an entry to markdown_easy.services.yml to give the hook class a service name, and specifyautowire: true.
9. In the hook functions in the module file replace the function code with a call to \Drupal::service() to call the hook service, and call the equivalent object oriented hook.
10. Autowired services require Drupal 10.1 or higher, so older versions should be removed from the .info.yml and composer.json files.

PHPStan was reporting Unsafe usage of new static.

Used example from this page → to update phpstan.neon.

I think the use of plugins makes the code much cleaner and maintainable.

However, I'd like to propose a few small changes.

First, the Flavor dropdown was populated with:

      '#options' => [
        'standard' => $this->t('Standard Markdown'),
        'github' => $this->t('GitHub-flavored Markdown'),
        'markdownsmorgasbord' => $this->t('Markdown Smörgåsbord'),
      ],

This negates one of the big advantages of plugins, which allows new plugins to be added without needing to modify this module.

Instead, we should populate using $this->markdownEasyPluginManager->getDefinitions(), which allows new plugins to be included in the list without any code change. This would allow either contrib or custom modules to provide their own additional markdown flavors.

Also, the Markdown Smörgåsbord had an ID of markdown_smorgasbord, but the config setting is markdownsmorgasbord, which was requiring a match() to map them correctly. That's the only place an underscore is used in the flavor, and changing the ID to markdownsmorgasbord allows that to be simplified, and removes the need for the match().

I attended!

lostcarpark → created an issue.

lostcarpark → created an issue.

The hooks look good, but PHPStan is giving a warning for previous major, because the hook attributes are not known in D10.

I have added a before script to .gitlab-ci.yml to delete the attribute lines from the source file before PHPStan runs for previous major. This will only take effect if the Drupal major version is 10.

Currently the info.yml file contains:

core_version_requirement: ^9.5 || ^10.2 || ^11

When 4.0 released, D11 hadn't yet released, so 9.5 was still supported.

Merged successfully.

Thanks for working on this.

Looks perfect, and all tests are passing.

Moving to RTBC.

I have reviewed the test change, and it all looks good.

I ran the "test-only changes", which rolls back all changes that aren't to tests, and runs the changed tests against the unchanged module code.

This failed as expected.

Moving to RTBC.

lostcarpark → created an issue.

lostcarpark → created an issue.

lostcarpark → created an issue.

This shouldn't cause problems, though I've messed up composer changes before...

Gitlab seems to be having some issues at the moment. Will check the pipeline and rerun if necessary.

lostcarpark → created an issue.

That sounds like a good strategy to me.

• Extend the test.
• Verify it fails without this change.
• Verify it passes with the change.

The new tests look pretty good!

I will do some local testing, then merge.

Hoping to get a release out this weekend.

I missed a PHPCS error about an extra line break at the end of the libraries file. Fixed now.

Thanks for your help, @sandeep_k.

Thank you for reviewing. Merging.

Merged successfully. Thank you for working on this.

StyleLint was failing, but was not related to this change, and already fixed in another issue. I have rebased this issue, and StyleLint and all tests now pass.

The new test looks good. I will give the change a final check over and merge soon.

Thanks for your work on this.

lostcarpark → made their first commit to this issue’s fork.

Merged and all tests pass, so moving to fixed.

I need to carry out a final test, but I think this issue is resolved by 🐛 Broken URL for editing a scheduled item Active , so it should be good to close.

Thanks @vitaliyb98 for your work on this!

Change merged successfully.

I've done some testing and verified tests are passing. Ready to merge.

Thanks for working on this.

There are several issues covering multilingual support, and I'd really like to merge them. However, I feel that we need some test coverage. I worry that someone (probably me) will make a future change and forget to test multilingual features and break something.

So I think it's important to have basic test coverage.

If you could outline the steps to test from a fresh Drupal install, that would be a great help.

Thank you for working on this. Patch files are deprecated, so patch will need to be converted to a merge request (or the existing MR !24 updated).

If someone has time before I get to it, please go ahead.

I feel this should also have some test coverage, as a lot of developers working of future features may not test on multilingual, so there should be at least a basic test of multilingual functionality, or it's likely some future feature release will break it.

Seems a nice straightforward fix, and works for me in test.

Sorry I haven't looked at this issue. It would be great to try to move it forwards. There are some outstanding questions in #9, that it would be great to get opinions on.

Merged successfully.

I have checked the change and it looks perfect.

There is a failure of phpstan (previous major), but that is not related to this change, and I've opened 📌 Fix PHPStan for previous major Active to resolve.

Will merge now.

lostcarpark → created an issue.

I attended.

That would be great. A good start would be to outline the steps a Kernel test needs to follow. It might give someone a head start before you get to it.

Looks good!

Any thoughts on how we could add test coverage for this? I'd be happy enough to merge as is, and hopefully add test coverage later, but if we could easily cover it off, it might be worth putting some effort in.

Thank you! The problem with pushing changes when I'm tired! I did a git status then pushed the files that had changed, but forgot the new files. Hopefully fixed now.

Also present.

Merged into 4.x branch.

Moving to fixed.

Icon added as shown:

Ready for review.

lostcarpark → created an issue.

Thanks for this. Omitting the version makes sense. All tests pass.

Rebased for latest 4.0.x changes. Tests are passing.

lostcarpark → made their first commit to this issue’s fork.

I have rebased to include 🐛 Unable to update a scheduled record Active , and verified tests are now passing.

Ideally someone else should move to RTBC since I developed originally.

I have merged the latest change. I would love to add test cases to fully cover this, but don't want to delay other pending changes.

There was a PHPCS issue reported on previous test run, but it went away when I retested, but a Stylelint issue appeared instead, so I tweaked some selectors to fix.

I have run tests locally and carried out a manual test. Ready to merge.

Tests are now passing on 1.4 branch.

Aim to continue development on this soon.

Merged and moving to Fixed.

As this is adding a step top the pipeline, and the pipeline is passing, I'm happy to merge this.

It would make sense to use <!--break--> to end the text if it comes within the smart trim limit.

For example, if Smart Trim was set with a limit of 100 characters, and a <!--break--> was present at character 50, then Smart Trim would return up to the <!--break-->.

However, if the <!--break--> came at character 150, then Smart Trim's normal rules would apply, and only the first 100 characters (adjusted for word breaks) would be returned.

As an extra module is required to support the <!--break--> with CKEditor, I don't think this is widely used, I wonder should there be a config option added to enable support of <!--break-->? If selected, text would be trimmed at <!--break-->, if it appeared within the trim length. If not enabled, <!--break--> would be treated as a comment.