Root Title, Min. Level, Limit and Dept

Thanks for all this work! I'm a bit swamped at the moment, but will try to review it soon. Reviews from other interested users would be very much appreciated, of course.

Splitting large code changes into smaller patches is generally a good idea, as it makes patches smaller and easier to test and review. The point is to be able to commit the patches individually while keeping the module is a working state. Some of your patches, however, do nothing by themselves (e.g. adding new functions not called from anywhere), and some depend on other patches. I would not commit a patch that does nothing, and I cannot commit a patch that depends on code that's neither in the module nor in the patch itself.

If you want to get a specific feature into a module, try not to change stuff that's not directly related to that feature (the unrelated changes belong to separate patches).

Note that function documentation has to use Doxygen formatting.

In the future, please try not naming the patches with version numbers that look like official version numbers... That could confuse some users.

I did not review the patch yet.

I tried your patch and it seemed to work ok. Could you add a "Depth" option to specify how many levels deep the trimmed menu should display at any given time.

For example, if I specify a "minimum level" of 2 and a "depth" of 3, then the menu trim block would only start trimming menus at level 2 of the menu structure AND only show 3 levels of hierarchy at a time. This would help when managing large hierarchical sites with deep menu structures. I don't want a huge left sidebar that shows a big hierarchy, I only want it to show 3 levels at time to keep it clean. I know you can hide it with css, but seems like you should be able to handle everything serverside.

The "Local Menu" Module provides this depth functionality, but it is only for Drupal 6.

What do you think?

That is correct. You could have a self maintaining sidebar. Man that would be cool. Also, is there any way you could post the modified module along with the patch files? Currently all the modified files seem to be a little spread out. It would make it easier for people to test the modified module out.

Thanks for your work, looking forward to seeing any changes.

@DynV - I'm not sure I fully understand what you are saying. In studying Drupal, I found Drupal 5 lacking a module that limits menus from showing too many levels of hierarchy at one time. It is easy to style with CSS a vertical side menu that shows three menu levels deep, but it is hard to style a vertical side menu with infinite levels of hierarchy. Menu trim works, but there is no automation, you have to trim by hand.

The goal I would like to acheive is to have two menus, one horizontal drop down that only shows up to three level of hierarchy and one left vertical sidebar menu that only shows two or three level at a time. Each menu would be in sync with each other. On my homepage I would like to show the horizontal dropdown menu only. Then when someone clicks on a menu item in the dropdown it would take them to that page with that menu item deactivated and highlighted in the dropdown menu AND now show a menu in the left sidebar starting from that point in the menu tree, but only show whatever the maxium depth is set to.

Here is a website that is similar: City of Lancater and http://tv2sport.dk/cykling

I have attached a jpeg picture of how the "Local Menu" module automatically trims my menus in Drupal 6. The bad thing about the Local Menu module is that only supplies one block.

I envisioned a module that would create a block for each menu where you can specify settings per block. Of course I would like it to do the same thing that the menu trails module does by allowing you to highlight were you are at on the site. Each block would have a "maximum depth to display" setting.

That picture illustrates what I was thinking.

Let me know what you think.

@DynV - Both examples would create a self maintaining menu. The image just shows my example menu that was produced with the Local Menu module. I inserted the breadcrumbs to show you at what level the breadcrumbs start to appear and what there value is a that level. The "horizontal dropdown menu" I was referring to is displayed on the example sites using Javascript to dropdown the menu items.

So, all together you need these options:
"Replace menu title" - this might be the same setting as "Root Title"
Level at which to start trimming the menu - this might be the same setting as "Min Level"
Maximum depth of menu items to display after being trimmed
Limit - Won't allow trimmable items to trim under its menu minimum level.

What do you think about all that?

@Dynv - Is there anyway you could post the modified module? Maybe you could zip it or tar it? That would be helpful for others to test it.

Great Job - I'm looking forward to testing it.

@DynV - How can I keep expanded menus from going past the depth setting? Is there a way to disable the "expanded" setting on menu items to avoid breaking a layout.

For example:
Say I have a menu with a depth setting of 1, yet all of it's menu items are set to "expanded". It will keep displaying all of the expanded menus thus breaking a layout that only allows for menus that are one level deep. I guess that could be a feature request. "Disable expanded menu items"

Everything seems to be working good now. I did have some weird troubles when testing out this module. It would actually bog down my computer and then display a white screen. I had to delete the menu trim variable and then reinstall the module and that seemed to fix the problem. I think I might have used some settings that were not logical.

Also the .install file for some reason wouldn't create the tables in the database for me. I had to manually set the tables up. I am using MySQL.

Thanks for all your modifications to this module. This is exactly what Drupal needs for very hierarchal sites. Will these modifications eventually be rolled into the menu_trim module?

tracking

@DynV FYI.. So far, everything seems to be working fine with the testing of the module.

EXCELLENT module, thank you - If I ever get around to posting the corporate brochure recipe summary, this module's part of it. . . and BTW this functionality IMO should be core.

I would also like to "disable expanded". I'm using Menu Trim to display tertiary+ nav levels vertically in a sidebar block (in coordination with horizontal Primary/Secondary, all in one menu tree - I want to be able to let the tree go deep, while showing just the relevant two or three levels there, letting the breadcrumb trail bridge between the always-visible top-nav and the section-specific sidenav when the intermediate levels aren't showing. Haven't gotten past six levels yet, but so far so good. . . FYI creating my nodes/paths/menu items using Node Hierarchy, helps keep everything coordinated and the two seem to play well together.

However I'd like to also have dropdowns from the Secondary, which the site visitor would probably use for initial exploration. These allow for quickly selecting specific items deep in the hierarchy without the default Drupal behaviour of forcing a click-reload cycle at each expansion point in a deep hierarchy. The sidebar menu then becomes more of a user-orientation feature, giving them a context not provide by the drop-down once they're at the node itself.

I'm currently looking at how Tapestry does suckerfish "mostly CSS-only" dropdowns, but that requires all levels to be flagged Expanded, will also be looking at Nice. Current workaround is to duplicate the whole tree in another menu, but I sure don't want to maintain two! Other way's to code a static custom-expanded version of the tree for a few key points using block visibility, and let Menu Trim handle it elsewhere, but again, a maintenance headache.

I think being able to tell MT to ignore the Expanded flags would be an easy way out.

By reading the current title, "+1 level", I have no idea what this issue is about.

Tip: When an issue is important to me, I usually try to catch the attention of others through a proper title.

The patch "level4-orig.patch" at #22 doesn't apply against HEAD. There seem to be some sort of problem with newlines, but some original code in the patch also looks older than the current HEAD version.

patching file menu_trim.info
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file menu_trim.info.rej

patching file menu_trim.install
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file menu_trim.install.rej

patching file menu_trim.module
Hunk #1 FAILED at 2.
Hunk #2 FAILED at 61.
2 out of 2 hunks FAILED -- saving rejects to file menu_trim.module.rej

patching file README.txt
Hunk #1 FAILED at 1.

Also, not the cause for the failures, but filenames in the patch are absolute paths. Using relative paths would make patching easier and faster.

You may get the version numbers from CVS. For example, the latest menu_trim.module is 1.3, while the patch shows 1.2.

Just checking-out the HEAD branch with CVS will get you the latest version of all the files (drupal.org's CVS repository is open to anonymous users (read-only)). You may also find that using 'cvs diff' is more convenient than the plain 'diff'.

Also make sure that all files are in Unix format (check both your text editor and your CVS client). When in doubt, you can always use 'dos2unix' to convert them before diff'ing.

@DynV: Unfortunately, I do mind. First, with or without a patch it would be backward to review code that's not based on an up-to-date version of the module. Second, drupal.org's patching & reviewing process is designed to be quick and convenient; reviewing packaged modules is not. Third, I have already rejected your co-maintainership request, which rules out a commit. CVS access to a module on drupal.org is an "all or nothing" thing with no per-branch permissions. I'm not confident enough at the moment to grant you this. Also, I fail to see how letting you do your own thing in a separate branch would be so different from a fork of the module.

I hope that you'll understand and try to work with the normal patching & reviewing process. It is that process that drives Drupal's development. Yes, that's sometimes frustrating to see our hard work languishing in the issue queue. It happens to every Drupal contributor. Quality stuff that attracts strong community interest, support, reviews and contributions usually gets the attention of maintainers no matter how much time they have — and most, if not all, maintainers have much less time than they'd like. In the end, Drupal core is proof that the process helps build high quality software. Also, I'm sure that many contributors could attest that the process favors learning.

I'm not sure to understand the process you are proposing. The process on drupal.org is well documented. For all I know, the last patch fails (see #34). If you can't even merge your changes with the current version to make a valid patch, I don't see how you could properly handle a "merge of two branches" (which would be the same thing).

Enough time wasted — let's discuss code, not process. And to do that we need valid patches.

Regarding your questions:

1) In order to generate a valid patch, you need to compare your changes with the latest version of the module. A valid patch is one that applies when the patch program is ran with the patch against the HEAD version of the module.

2) It is not complicated to create patches. Just run diff or cvs diff against your version of the code and the HEAD version. I see no reason to skip the patch process. It takes much more time explaining all this than making patches. There is no complex re-engineering; this is a small module, about 10 Kb including the README file!