Closed (fixed)
Project:
Patterns
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
27 Apr 2009 at 12:03 UTC
Updated:
26 Mar 2010 at 13:00 UTC
Jump to comment: Most recent file
Comments
Comment #1
tamasd commentedI think this is not visible right now
Comment #2
dman commentedYeah, me too. I got HEAPS of notices when trying to use patterns in an install profile.
The above patch has a few tabs in (naughty when doing compliance ;)
Here's my version of the same fixes, rolled today. I got EXACTLY the same points of notice as Yorirou .. plus one more.
Comment #3
dman commentedRe-roll.
It's not much fun to work with if my dev copy is going to remain out-of sync with the CVS copy. This means I can't submit any other patches until the code style is cleaned up.
Comment #4
joestewart commentedthanks for trying to clean these up.
Is this portion in there a related bugfix too?
Comment #5
dman commentedIt's a fix, it also triggered a strict warning, but to fix it meant repairing the logic also. $result->file is always undefined.
Looked like a typo that got overlooked to me.
Review the logic and see what you think, but I can't see how a db result object could ever have a ->file property, or why you would loop over all $patterns but do nothing with the values found.
This is one of those cases where turning on strict actually does expose some corners of the code that needed review. I think.
Comment #6
dman commentedRe-roll against current dev.
This one now also includes a feature I was trying to clean up the code before adding. During development and testing, it was confusing to see whether the pattern being used was really the most recent one. The caching was getting in the way, so I added a button to 'flush' the remembered pattern in the catalog and ensure that the pattern being run was really the most recently edited file one, not the database-cached one. It's an extra 'action' on the /admin/build/patterns page.
I don't use the UI editor a lot, I use a text editor that saves files as files, and the web interface is for tweaks, but not for development and iterative testing.
Ideally, this would be a separate feature request. But the code style/E_ALL fixes were required first, and now are ill but inextricable from my branch of the code. The previous patch with only E_ALL fixes probably will still apply.
(Patch may be messy with whitespace changes - I also inadvertently stripped a load of trailing spaces from the line endings)
Comment #7
vaish commentedI committed the part of the patch dealing with E_ALL compliance. However, I skipped "flush pattern" part because I would love to preserve pid (pattern ID) between the flushes. I'm sure there must be a different approach to flushing that will take care of that requirement.
@dman - thanks for your work on this patch and patterns in general. Very much appreciated. I haven't been able to work much on patterns for a long time but I really hope I'll be able to change that in upcoming months and spend more time on issue queue and new development.
re #4 and #5 - yes that was a typo that got overlooked. Thanks for catching that.
I'm marking this issue as fixed. Let's deal with "Flush patterns" feature request in a separate issue.
Thanks to everyone who contributed to this patch,
Vaish
Comment #8
vaish commentedIssue with patterns in the database not being refreshed after file has been modified was actually a bug. I just commented out problematic piece of code until I'm entirely sure that there are no side-effects. In any case, now it's enough to just refresh patterns listing page after pattern file has been edited and new pattern version will be loaded and stored in the database. In case this pattern has already been enabled you will see that link for running the pattern now reads "Run Update" instead of "Re-run".
This is the relevant commit: http://drupalcode.org/viewvc/drupal/contributions/modules/patterns/patte...
Comment #9
dman commentedThanks, that's probably what needed to be done then. I wasn't sure that the previous - awkwardly cached - behaviour wasn't intentional, or I was just using it in an unexpected way.
When working on these things, I just preferred to be using a file/code editor, not a webform. I thought maybe that workflow was against expectations or something.
Yay then. Thanks!
Comment #10
vaish commentedThis caching was totally unintentional, and I remember well it wasn't happening before. Not sure what and when changed that introduced this bug. Btw, I'm also always using code editor and was equally annoyed with the caching as you.