Add prefixes / suffixes to mysite items, and determine if they should open in current or new window/tab

I like the basic settings approach -- being able to override prefix, suffix, link type in the UI -- but I have a few issues. (Note, this is based on reading the patch, not applying it).

1) The patches are out of date. That is, they are written against older versions of HEAD. They should be refactored against the end of the 5--2 branch. For example:

-  drupal_set_message(t('Could not find aggregator data'), 'error');
+  drupal_set_message(t('Could not find data'), 'error');

This reverts a correction that I made recently that makes debugging a lot easier.

2) I like the form_submit piece -- stuffing all the type settings into a single array seems like a good idea. But you will also store the op, token, submit, and form_id values the way it is written now. I typically unset() the values I don't want before processing. Something like:

  $temp = $form_values;
  $unset = array('form_id', 'token', 'op', 'submit');
  foreach ($unset as $str) {
    unset($temp[$str]);
  }
  # now process what's left
  foreach ($temp as $key => $value) {
      ....
  }

3) The plugin patch is a little inconsistent. In aggregator, the 'link_target' is coded into the array. In other cases, it is merged from the settings. Consistency here. I would not add 'link_target' to the $type array. Assume that cases other than an empty target are special.

4) Be forewarned that I'm rewriting the menu from scratch for 5.x.3. In order to allow multiple pages, I have to change some of the variable handling to accept an argument allowing for multiple pages per user.

-----

Those points aside, I love the fact that your team wrote and submitted a patch for this behavior. If this module is going to move forward, we need contributors other than me.

Yes, the locked items code is a little flaky and hasn't been stress tested.

And I was wrong before: patch new features against HEAD. I branched 5--2, which means its in bugfix only mode.

So patches for these go to:

Bug fixes to content locking => 5--2.
Settings for prefix/suffix => HEAD
Changes to content locking => HEAD

The current menu changes are simply on my dev machine; I have some serious testing to do before I commit anything. When menu changes hit, they will be committed to HEAD.

Hello!
Thank you for your comments!
There is new patch. I took HEAD version of module and added all changes again, so now all should be ok.
About settings form submit. I use there filter by key of form value, so don't see why i should use there unset(), only values with key like 'mysite_type_'. $type .'-'.$setting_name will be saved, so we can add other fields on form like hidden field 'type', for example, and we don't need change this function.
Please, let me know if you see there better way.
Thanks!

@tdekhnich, yes, filtering by the $key should work. I haven't tested the patch; I just read it.

Using unset() would just strip the $form_values() that are there by default. Under your method, anything not explictly named 'mysite_type_' . $type will get removed.

Two ways to solve the same problem.

This new feature will be rolled into the 5.x.3 release. I think it's great.

With luck, I'll be able to test this patch next week. I'm about to depart for DrupalCON Barcelona (going to Madrid first).

If we're being really good, then the plugin changes will also be applied to elements in the 'contrib' folder. I can do those if you don't want to alter the patch.

Looks like there is a line-break incompatibility in that patch. Look at the section for post.inc.

The files should all be Unix-style line breaks. Looks like your are Windows, so all the returns were transformed to double-returns. As a result, the patch rewrites the entire file in that case.

The original post.inc code from heebiejeebieclub was malformed, and I didn't correct it.

I'll have to reload post.inc into HEAD so you can reroll the patch.

I just committed the linebreak fix to HEAD and 5--2.

separate patch for mysite.module

separate patch for types

Hope to test this week. Thanks!

I finally tested the patch and it works great.

I cleaned up a few code style elements, added the routines to the items in the plugins folder, and will commit to HEAD.

Only code error I saw:

You don't need to call cache_clear_all() after running variable_del(). See http://api.drupal.org/api/function/variable_del/5.

Thanks to everyone who worked on these patches. This is the first new contributed functionality for the module.

Now in HEAD.

There was a logic bug that caused the Theme configuration options to disappear.

The attached patch has been committed to HEAD.

See http://drupal.org/node/219879

See http://drupal.org/node/219879 for the patch.

This has been committed to HEAD.