Ontario, CA 🇨🇦
Account created on 22 April 2009, almost 16 years ago
#

Merge Requests

More

Recent comments

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

phpunit now passes on both branches.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Tests are passing for 4.x. They are failing for 8.x-3.x, but they are failing on the dev branch too. The merge request here fixes phpcs on that branch.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland made their first commit to this issue’s fork.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

All tests are now passing except cspell and eslint.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland made their first commit to this issue’s fork.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks.

Can we get a new release, with PHP 8.4 support? Other modules depend on search_api, so this is holding up PHP 8.4 support in those module too.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

It seems likely that $rows doesn't have to be generated either. That was put out of use in #3084714: Render with Views API more properly. , commit b9e2350.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland made their first commit to this issue’s fork.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Rebased; phpcs now passes.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please add comments explaining what each part of the new function does.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Are the two changes to #markup necessary?

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Changes are correct. Tests pass on max PHP.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This should include a list of all allowed values for #type and what properties they have. There was excellent documentation like this for Drupal 7.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Tests are passing on max PHP.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Rebased. phpunit passes.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I have rebased. Tests do not pass.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please try the fix in that branch. We may be able to cherry-pick it onto 6.2.x.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch. Please put it in an issue fork and merge request.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please put the patch into a merge request targeting 3.0.x.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please put the patch into a merge request targeting 3.0.x.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I think you can do this with an entity query. Query for terms that have the given term as a parent.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This patch contains the current state of the merge request.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This patch should go into an issue fork and merge request.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I would find it more convenient to leave the config as it is until the next time the config is saved. It defaults to the current values so nothing changes.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Re "these tests were extremely broken prior to this merge request": The current merge request adds the .gitlab-ci.yml file. I suggest committing 📌 Add testing configuration Active . This adds .gitlab-ci.yml and gets the tests passing. Use that issue to get tests passing and then ensure that they keep passing in the current issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

@#18 Does the patch in the merge request fix it for you?

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

No reply in 2 months.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Done

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch.

Please put the patch into an issue fork and merge request.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This makes sense to me. Can you add a test?

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please fix the coding standards issues. Can you add a test?

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch. This is a good idea.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Are you able to add a test that demonstrates this? That would help to get it solved and prevent a regression.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Link issues in summary.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I know the code in #3 would not work. I wrote that as an example of what I would like to be able to do in order to avoid executing the View twice.

I think being more efficient is more important than not looking weird.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Why the call to function_exists()? mb_convert_encoding() has been in PHP for ages.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Why the call to function_exists()? mb_convert_encoding() has been in PHP for ages.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks. I would rather it not be on the page at all.

I'd like to switch this to a feature request: Add a function which returns a printable value if there are results in the View and otherwise returns NULL. That would allow the new function to be used in place of drupal_view_result() in code like in #3.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

So, is my code snippet the recommended way to do it?

To save rending the View twice, I thought that something like this would be possible:

{% set view_content = drupal_view_result('related', 'block_1') %}
{% if view_content is not empty %}
  <section>
    {{ view_content }}
  </section>
{% endif %}
🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

git bisect helps you find it. If it's broken now but there is some previous version that worked, it will help you to search for the first commit between the two where it is broken. git bisect will checkout one version after another and you just have to test and tell Git if the current one is good or bad.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Can you tell which commit caused this? You can use git bisect to figure it out.

💬 | FillPDF | Use case
🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

One use case: Complete a Webform, download a PDF filled with values from the Webform, sign it, and mail it in. This gives a signed paper form with data already in your system.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I have made more fixes and phpcs now passes. One of the changes renames a property, which could cause issues for anyone who extended this class. That change could be left-out for now and an exception added if you want. Please let me know.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Did you add a visibility declaration (like protected) to all the properties? It should be added only to those properties that are defined in this class. Any inherited properties should not get a visibility declaration.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Interesting. What if you are on the feature branch that tracks the merge request branch in the issue fork and just run git push --force without any other params? That is what I usually do.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

liam morland created an issue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Testing is now enabled.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

You could rebase the branch and force-push it.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

MailManagerInterface::mail() returns an array of information about the message and "the 'result' element will contain the success indicator of the email". For your code to work, it would need to return FALSE when mail fails. It doesn't.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

I don't recall the problem that I was facing last May, just that I had traced it to the missing newlines.

POSIX says they should be there. Is there any standard that says they should not be there?

If a CSV library does not work properly when the newline is there, that sounds like a bug in the library.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Ending every line with newline is a requirement of POSIX. The definition of a line is "A sequence of zero or more non- characters plus a terminating character." So, unless the file contains zero lines, it will end in a newline.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch.

It seems to me that the default separator should stay the same so that existing installs do not change.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Sorry, I can't quickly find examples. It does make sense to me. On the other hand, as long as the versions for which support is being dropped are no longer supported, perhaps it doesn't matter. This merge request puts the minimum at Drupal 10.3 and that is the oldest supported.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

In other issues, people have suggested that dropping old version support should not be done in a patch-level release. To follow that, these changes should go in 2.0.x. There could be a 2.1.x which drops supports for CKEditor 4 and perhaps Drupal 10 as well.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please confirm that the problem does not exist in 6.3.x.

I have removed the property. I expect that will fix it. Please test.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This will probably require a ticket in the Drupal.org infrastructure queue.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

You can checkout the branch of the merge request, run git reset --hard 6.3.x, make your changes, then git push --force. That will re-use the current merge request. It will automatically create a tag pointing at the old commits so they are there to refer to. Nothing gets lost.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the rebase. By the way, you can just force-push to the same branch as the merge request instead of making a new merge request. It will automatically create a tag pointing at the old commits, so there are there to refer to. Nothing is lost.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Please rebase onto the latest 6.3.x; that will fix some errors. The comments added in src/WebformRequest.php should not have an empty line between the @param statements. Unless this method doesn't match what is in WebformRequestInterface, it can use {@inheritdoc}.

I note that the tests still don't pass on PHP 8.4. This may have to wait on other modules updating.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

Thanks for the patch.

I see a yellow triangle here, lines 244 and 253:

https://git.drupalcode.org/project/webform/-/merge_requests/585/diffs#49...

There cannot be a default value for any param that is followed by a param that does not have a default. Example:

public function getUrl(EntityInterface $webform_entity, ?EntityInterface $source_entity = NULL, $route_name, array $route_options = []) {

$source_entity has a default, but $route_name does not. To fix, remove the default from $source_entity.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This patch as-it-is has the problem that it breaks any code that extends MessageNotifierBase.

🇨🇦Canada Liam Morland Ontario, CA 🇨🇦

This is a very big patch and I am feeling some pressure to have full release for 6.3.0. Perhaps we should put this onto a 6.4.x branch.

Production build 0.71.5 2024