Closed (fixed)
Project:
Directory
Version:
5.x-2.0
Component:
Code
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
21 Jul 2007 at 14:58 UTC
Updated:
6 Aug 2009 at 21:06 UTC
Jump to comment: Most recent file
Changes:
- no more crucial logic in the theming functions (e.g. theming the tree is now MUCH easier)
- refactored pretty much every line of code, with the exception of the _directory_term_count_nodes() function (which was called directory_taxonomy_term_count_nodes() previously), _directory_select_nodes (previously called directory_select_nodes() and the admin settings form.
- support for filtering by content type
- removed the alphabetical directory feature, it was broken anyway
- used the slide up/down effect for showing/hiding a vocabulary
- also implements http://drupal.org/node/160688
- ...
P.S.: now it's 1/3rd less code to maintain: 670 lines instead of 1000.
| Comment | File | Size | Author |
|---|---|---|---|
| #1 | directory.zip_.txt | 21.22 KB | wim leers |
| refactored.patch | 57.96 KB | wim leers |
Comments
Comment #1
wim leersAnd the entire module, in .zip format. You may want to use this instead, the patch is of little or no use because there are so many changes.
Comment #2
beginner commentedThis is not helpful at all.
1) you remove code that I explicitly said I wanted fixed, not removed.
2) you retain features that I want to get rid off.
3) you add features and mix them with bugfixes.
4) you present the whole in a large, unreviewable patch.
I am glad that you have been able to complete your assignment for your client. However, you are not helping me at all, and providing this for users is not helpful either, because they may be stuck with an unsupported fork of directory.module.
Comment #3
wim leersHow this is not helpful? It's refactored, the code makes sense again and is properly documented and is according to the code guidelines. A refactored module is per definition not easy to review.
1) This doesn't make sense. The only removed feature is the alphabetical listing. This can be added again easily.
2) I retained features that *you* want to get rid of. But I told you why it was *necessary* to keep those features, even if it is at least for now. You also have your "temporary-until-the-core-patch-gets-commited" node select function. So why shouldn't we keep our own node listing pages? Not to mention you would not be able to filter by content type if you would reuse taxonomy.module's node listing pages.
3 & 4) Of course, because it is a complete refactoring.
A patch is *never* unhelpful. At worst, nothing of it can be used, but at least people can learn from it. And I posted this patch here, so we could work towards a consensus. I'm not trying to fork directory.module.
Did you even try the patch?
Please download the .zip and give me some real feedback about the new structure, and if you will, the code. I'm sure we can work something out.
Comment #4
summit commentedHi,
I tried the refactured module. From scratch I got all my taxonomy terms first letter from every node. So I got at the first page
A
A
A
A
etc..
I would like to get:
- A (and if I click on the A, get all the terms with an A, and then if I click the term, all the nodes which are under this term.
- B
etc..
I am not getting this fixed.
Also not with enabling -one of- taxonomy term.
Greetings,
Martijn
Comment #5
wim leersI have not the *slightest* idea what you're talking about. Alphabetical listings have been removed from this version.
Also, if you don't know how to use the issue queue, then don't. You've changed the title to something completely irrelevant.
I'm leaving this as CNR for whenever beginner or another maintainer decides to give this a look, but my hopes are low.
Comment #6
wim leersClosing due to lack of interest.