Merging with Metatags

I don't really have any problems with combining these projects, if you really wanted to do this.

Combining the two projects is what I was suggesting to do. I think that both the projects would take vantage of this.
I was thinking to allow the use of tokens in the definition of some meta tags, and as Meta Tags by Path already support the tokens, there would not be duplicated code between the two projects.

Merging the two modules sounds like a great idea. But in the meantime, I suggest fixing Meta Tags by Path to make them actually work together.

I tried the latest dev versions of both modules and had to come up with this patch for path metatags to show up. First part of the fix picks up the correct tag name for Meta Tags by Path form and the second one is a hack to get around inappropriate use of drupal_alter by Metatags.

This patch will likely get obsoleted by full merging, but in the meantime I suggest it to be reviewed and committed.

nodewords_bypath_nodewords_tags_alter_alter() should be nodewords_bypath_nodewords_tags_alter().

EDIT: the code in Meta tags is wrong; the string passed to drupal_alter() should not contain '_alter'; thanks for pointing out that.

Nope, for this patch to work, you need _alter twice - try it out. That's because Meta tags call to drupal_alter, passes wrong 'nodewords_tags_alter' param, which should have been simply 'tags'. The function name called by drupal_alter is always [module]_[param]_alter.

I noticed that Meta tags makes another call to drupal_alter with similarly looking wrong param. Because I am not that familiar with specifics of these module's logic I decided not to touch it. You might want to just use my patch as a starting point to update both modules.

Really, both modules should be rewritten and if drupal_alter is still used it should make that call correctly.

The code executed by drupal_alter() is the following (this is the last part):

The function doesn't add the module name to the parameter passed, but it appends '_alter' at the end of the string.
The string passed to the function should not have a ending '_alter', and that is the reason I am saying Meta tags code is not correct.

@Kiam: Have you actually run the code? I did and only the variation with _alter_alter works. You already have one "_alter" inside $type. Look carefully.

What I am saying is that the code you find in Meta tags is not correct; the string passed to drupal_alter() should be 'nodewords_tags', and 'nodewords_tags_list'.
The strings the actual code passes to drupal_alter() are not correct, nor the one I thought to use.

Consequently, you are basing your patch on code that must be changed because it is not the one I had in mind; while I was writing the code, I though to different alternatives, but I didn't change the strings to the correct ones before to make the last commits.

I agree, the code in Meta tags is not correct. My patch is simply a temporary fix I made for my own needs. Feel free to rewrite/apply for both modules.

The patch reported here needs to be updated because Meta tags code has been corrected.

Anyway, it seems that the maintainer already expressed his opinion, and he doesn't seem interested in this topic.

Which maintainer? In any case, feel free to update and commit the patch...

I meant the maintainer of this project. As far as I can see, the maintainers are univate, and Shannon Lucas; univate replied on this report, and I understood he was not interested in this (I could be wrong, anyway).

I think he only says he has no problem with you combining projects... I do not see any comments on my patch.

In any case, here is the updated patch to work with Meta tags latest.

the reply he gave has been "I don't really have any problems with combining these projects, if you really wanted to do this"; I am not sure if he really meant "if you want to do this", but he didn't reply to the comment I added in reply to his comment, then I guess he is not interested in this.

Currently I am maintaining this module, I did the port to D6 and took over at the time as maintainer from Shannon (who originally authored it), I recently noticed that it appears he transfered the ownership of the module to Todd Nienkerk (I'm not sure what the reason for that was?).

The way I see it - including the functionality of this module directly in nodewords itself is really a decision for the maintainers and users of nodewords, as I previously stated I don't really have any problems with it, so you wouldn't get any objections to it from me. I didn't see any other follow up questions that you wanted me to respond/comment on? If you wanted to add me as a developer of nodewords project to help assist maintaining this module, I would be happy to continue being involved.

My opinion is that smaller modules providing specific functionality are usually better then trying to squeeze a lot of features in the one module/project.

To me it make sense to combine if the functionality provided in this modules is almost always used by everyone using nodewords - usage stats suggest about 4% of nodewords users are also nodewords_bypath users. Not sure if those statistics are significant?

Reasons to not combine include not wanting to force people to have to have this module installed if they don't need it.

I'm not really sure what is required to combine these projects? can we get the cvs projects combined? or do we just import the current version of nodewords_bypath into nodewords, loosing any history?

One of the comments you made in another issue was not being able to move to 2.0 version because of a rewrite being done for a new version of nodewords, would it make sense to focus on that new version for these changes as well?

The main reason I was proposing to merge the two modules is to eliminate the dependence of this module from a function it uses, and which should be considered as a private function (as it is, following the coding standards normally followed by PHP4, which doesn't have a keyword for private functions).

We can find another solution for this; in example, we can create a Meta tags public function that can be used from this module without any worries that the function will change in the future (at least for the 1.x branch I am currently developing; branch 2.x is not developed by me, and I cannot report how that branch will change).
It's evident that the Meta tags public API has not been developed enough to allow the integration of other modules (differently, the other modules would not call two functions that are private). I would like to change the actual situation, and I need a feedback from who is developing the code of the modules that depends on Meta tags; I don't want to take assumptions about the need of third-party that result then false.

I should report what I think to do in the Meta tags code, so you can report what you think right, or not.

• I would change the actual settings page, and remove the parts to set the global, and the front page meta tags; I would replace those sections with a group of settings pages that allow to set meta tags basing on some conditions (the conditions could be "the page is the front page", "global meta tags"). This would open the possibility to third-party modules to add additional conditions; one condition could be "when the page URL is one of the following".
  This would also make possible to use a single database table to contain all the conditions that must be verified to set the meta tags; to make an example, the settings this module is saving in its database table could be saved in Meta tags table as {"page", URL} ({"page", ""} would be used for the front page as it is actually done).
• I was thinking to add the possibility to use tokens to set the meta tags content; the tokens used would be given from tokens.module, and this would allow to use the content of CCK fields to generate the content of the node meta tags; when the meta tag is not associated with any node, it would be possible to use the global tokens.

Every feedback is welcome.
I would really like to make possible for this module to use a Meta tags API that so far has not been developed for third-party modules real needs.

I second Kiam's reasoning for merging. My perspective is purely as a user trying to get things implemented. With multiple modules, there are usually problems getting them to interact correctly. I wish that the time I spent my time debugging and submitting patches here, were devoted to tasks directly related to my own sites.

The way I see it, ability to add Metatags by path is just a feature, not standalone functionality. As an implementer I just want to add Metatags. I want a central way to manage all of them for the for all relevant Drupal entities, in every possible way (paths included), and I do not want to run into any conflicts between sub-features.

Please, let's merge them!

I like the ideas in #17, essentially that would make nodewords at its core more like nodewords_bypath, ie: a rule based module then node based.

I don't think nodewords should have been built around nodes, which are a low level content object, meta tags is a front end thing and depends on the way the content is being displayed not necessary the node(s) being displayed, e.g views/taxonomy/panels/context.

So If I was to redesign nodewords I would start with a base api module - you really just need one db table with three fields:
nodeword_id
name
value

I would also consider the following fields:
weight - gives a hierarchy of tags
append - a flag to specify if the tags can be appended to another tags or if they should just override other metatags if they have a greater weight

Each metatag_id is linked to a display hook function or module, examples include:
node
taxonomy
view
panel
context
bypath (the frontpage meta tags is really a special case here, even global metatags for the entire site could be described using the rule '*')

The above hooks could be in the one module or sub-modules of nodewords (so you can disable them if you don't use a particular module), you could even pushed the hook functions into the modules that they are related to.

This may need separate db tables for these display hooks, although you could combine some together

e.g. node, taxonomy, view could all use a common table
type - (e.g. node, term, view)
id - (e.g. nid, tid, vid)
nodeword_id (link to the nodeword table above)

other ones would probably require a separate tables e.g. by_path which can just use a similar structure to what this module provides.

When it comes to generating the metatags, the nodewords module would invoke the hook(s) and sort the resulting meta tags by their weight before displaying the result to the page.

Anyway that is just my 2c.

Also I just checked cvs and the 2.x branch of nodewords hasn't been worked on in 15 months?

I like the ideas expressed in #19.
I agree that the code should be split in more modules (this idea was expressed by the maintainer of Meta tags in a different thread), with nodewords.module containing the API. I still think that a central database table is enough, and the actual defined fields handle almost all possible cases.

I should also say that Meta tags now handles different types of meta tags, including the REV, and REL; there is the possibility for a module to define a new meta tags that will be rendered using a custom "template".

I am not sure if to handle views meta tags is something we really should do. The user interface used by Views don't use a separate page for each display defined for a view; as consequence, Meta tags can only use its field set to define the view meta tags, and it will try to apply its meta tags to each display of the same view, which cannot is not the correct thing to do as it can lead to wrong results. Maybe, a generic approach based on the page URL is the correct one (different displays have a different URL).