[D7] Target Filter URL

Thanks chakrapani, I've made the changes you've noted. It all seems to be working pretty well now. The PAreview conflicts somewhat with the formatting that running 'drush coder-format /target_filter_url' has put in place, but the few remaining errors that it cites seem to be ok. PAreview also indicates that the options for standard html target attributes should be wrapped in t(). That wouldn't work, so I think those errors should fly. I was confused about the version/changelog/license issue, thanks for pointing that out.

Ready for review again!

Project Reviews Noted ...I've been building sites for 13 years, using drupal for over two years now and on D.O for a little less than two, but this is the first time I've spent the time to go through other people's code in this fashion and ask that other people go through my code, it's pretty awesome -- a global community in action. Thanks!

The git clone is now included on the application page. Sorry about that.

I've gone through the PAreview again and fixed the indentations - which were originally auto-formatted by 'drush coder-format', so I think there may be some conflict there with PAreview. I realize this is odd because they run on the same code, but it's what happened. The remaining issues that it cites as far as code formatting I believe are irrelevant. I'm open to the argument that they are not irrelevant, but please let me explain what is happening and why I think it doesn't matter. Here's the PAreview (http://pareview.sh/pareview/httpgitdrupalorgsandboxjbranson2195787git-7x-10)

First) The t() on the options is not necessary in this case because they are standard html target attributes that do not need to run through a translate function, since they don't translate. I am concerned that running the values through translate would actually defeat the entire purpose of the project because it would mislead users as far as what attribute they are adding.

Second) I have no idea why it's kicking up :
FOUND 0 ERROR(S) AND 4 WARNING(S) AFFECTING 4 LINE(S)
--------------------------------------------------------------------------------
1 | WARNING | Line exceeds 80 characters; contains 260 characters
3 | WARNING | Line exceeds 80 characters; contains 83 characters
13 | WARNING | Line exceeds 80 characters; contains 138 characters
17 | WARNING | Line exceeds 80 characters; contains 190 characters
There is no white space and the lines are 1, 3, 13 and 17 are short.

Third) For the "247 | ERROR | Space found before comma in function call 267 | ERROR | Space found before comma in function call 286 | ERROR | Space found before comma in function call" This was formatted by drush coder-format and I'm running a php keyword so I think the whole thing is just confusing. As far I can tell, the way I have it formatted meets coding standards. If anything, perhaps the $pattern variable before the callback should be on its own line, but then that means that the rest of the callback function would have to be further indented and PAreview told me to delete indentations. ...In fact, while writing this reply I just tested again with the $pattern variable on it's own line and everything indented accordingly and PAreview gives the same errors and says to change indentation back to fewer indentations. So I reverted back to the commit that I ran through the PAreview included above at the top of this reply. I think it's solid, but if someone knows how to format it better, I'm happy to change it according to your suggestion.

Fourth) PAreview is saying that a parameter mentioned in a comment needs to have a defined type, yet the type is defined in the rest of the comment below. That whole comment is copied from drupal core's filter module, so I doubt it is really a big issue. I can define the parameter type but then it is redundant with the rest of the comment below it...and it is a comment...

Finally, here's where there does seem to be a relevant issue. I've set the branch to 7.x-1.0, I had preferred the 7.x-1.x-dev branch before while I was testing the code, but I guess for release naming convention it should be 1.0. Either way PAreview says:

The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226
* 7.x-1.0
remotes/origin/7.x-1.0
remotes/origin/HEAD -> origin/7.x-1.0

I tried running git origin --delete remotes/origin/7.x-1.0 and it tells me that the reference doesn't exist. There is currently only one branch showing up on the project page and I know I have deleted everything else. Does this have to do with my local repo on my computer? That would be odd. Any pointers?

Thanks for the reviews! I look forward to getting this project up and running for anyone who might want to use it!

Changing staus to needs review. Forgot that in above comment. Thanks again!

I've updated the readme.txt file. Clearly you can tell how much I was paying attention....because when I wrote comment #7 and said I don't know why PAreview was kicking up those errors about line length I was looking at .module while PAreview was talking about readme.txt. Idiot's mistake. Thanks for your patience!

The only issue that I see currently is the continued message about branches: The following git branches do not match the release branch pattern, you should remove/rename them. See http://drupal.org/node/1015226
* 7.x-1.0
remotes/origin/7.x-1.0
remotes/origin/HEAD -> origin/7.x-1.0

I've removed the tracking via git branch -d -r origin/7.x-1.0 and deleted the branch both locally and on the drupal.org remote (added 7.x-1.x again as default then delete 7.x-1.0, then readded and deleted 1.x). I checked out a new 7.x-1.0 and pushed it without the -u in the git command so there is no tracking. All of that and I still get the message on the PAreview. I'm at a loss for what to do about this, but I also really don't see why it's an issue if the project page only registers one branch.

Status still set to needs review. Please let me know if there is anything else I should be doing to fix this or if things are good to go. Thanks again for the help.

The issue is that the preg_replace_callback function only accepts one variable - the pattern match - so it seems like I have to make the function annonymous to get the attribute variable into the callback via the use keyword. Perhaps I'm mistaken though. Can the project page and documentation/readme.txt include a note that it requires 5.3+?

Added php version 5.3 added to info file. Per #13 above, I think this is the only way to go. Set to needs review. Thanks!

Thanks!

Thanks Klausi for promoting my first project. I've made the changes you suggested to the project page and set up a stable release. I thought that the form api handled validating options, so I'm not sure why I had the validation redundancy, but it's gone now.

Too bad about full git access, but that makes sense. If I make significant changes to this project can I pick up this project application for further review and possible approval? Otherwise, I'll include a link to this project and the application page in the application of whatever module I submit next for approval. Thanks again!