CKGeshi - CKEditor plugin for GeSHi Filter

I really appreciate your assistance with this review and overview. I'm doing my best to adapt to the drupal standards in a timely manor :)

I have pushed the required updates to the master branch - now contains only proper readme file, and tags have been updated properly.

7.x-2.0 branch updates
• I have ran the project through Coder, and updated it's documentation & spacing configurations respectively.
• I had placed it within a subfolder named after the plugin needlessly; files are now located properly in main directory.
• That 3rd party plugin is actually not 3rd party in this case. It is the true power of this module, I wrote it from top to bottom, and it is an extension of CKEditor module. Basically, CKEditor module provides a hook_ckeditor_plugin() function, which allows anyone to add a plugin to the ckeditor configuration & it's toolbar thereafter. So this is the "proper" way to include a ckeditor plugin, as opposed to dragging and dropping the plugin folder into the CKEditor modules "plugins" folder. :)
• I've renamed the .patch files properly to respect the module that they must patch
• I have updated the file 'default_options' to use the naming standard prefixed by module name.

Thanks again for your assistance and detailed review. This has helped me a lot in many ways (understanding Git, use of Coder module, naming of files and their proper locations). Truly much appreciated :)

Thank you very much for your interest and review.

• I'm definitely a newbie in terms of drupal code standards and git usage, so that is the reason for the lingering files in the master branch, and in the 7.x-2.x. I have removed them (extra .info files) from the branches.




• I agree, I too have never seen a .patch file included as mandatory for installation. Unfortunately, I have a feeling it may not change any time soon - the CKEditor module has a very large amount of issue tickets, and though it is a very stable and simple change, it may not be anywhere near being on their todo list.
  The only other way I can theorize to avert the need for this patch, would be to "hack" the ckeditor Drupal.ckeditorOff function with something such as:

  // JavaScript :
  var func_originalOff = Drupal.ckeditorOff;
  Drupal.ckeditorOff = function(textarea_id) {
     // these two logic-checks are performed by the original function, prior to performing anything
     // so they are duplicated here to preserve functionality:
     if (!CKEDITOR.instances || typeof CKEDITOR.instances[textarea_id]) === 'undefined') return;
     if (!CKEDITOR.env.isCompatible) return;
  
     // fire the event that notifies ckeditor is about to be destroyed (purpose of previous .patch file)
     CKEDITOR.instances[textarea_id].fire('beforeCKEditorOff');
     // and finally, call the original function:
     func_originalOff(textarea_id);
  };

  Though this would mean the .patch file isn't needed, it is indeed a hack.
  I'm honestly not sure which of the two would be the better solution, in terms of drupal-ethics and drupal-practicality. However for the sake of moving forward, I have integrated the above and removed the need for the .patch file. Updated the readme & project page to reflect this change. I'm beginning to see how useful git really is. :)




• I have ran the code through Coder and Codesniffer on my system, and pushed the latest to the 7.x-2.x branch.



Thank you again for your review and perspective on this, I think the above change is indeed a logical and smart step forward.

Thank you very much mitchell. No inconvenience at all, I definitely appreciate your swift efforts to push this module to a full state, the developer information, and your review. I have made the following changes in accordance & pushed to git repo:

• Renamed project title to ckeditor_geshi, updated references in code as well to reflect this single title
• Default branch has now been assigned to 7.x-2.x
• I've shortened the "INSTALLATION" section to reference the new INSTALL.txt file, removed some redundant information, and updated the README.txt to include the extra details that aren't necessary on project page.
• README.html has been removed, and README.txt updated to reflect project page changes
• Verified latest changes to CKEditor do not include Issue Request. At the moment though, no .patch is necessary, the hooking of Drupal.ckeditoroff function (provided by CKEditor) is so far 100% stable by my tests (thanks to KrisBulman for the idea). If CKEditor resolves the issue, I'll update this project, though currently is not a true problem for end-users

Thanks again for the review, assistance, and information. If any further changes are necessary to progress into full-stage, I'll do my best to promptly address them. I'm very excited to be releasing my first module and joining the developer-side of Drupal.org. I will do my best to be a good and useful member to the community, starting with some module reviews :)

@ GeduR,
Thanks for your review and assistance with this. I was not able to reproduce the problems you described, but I know for a fact that it is due to the Javascript code.
I wonder if you are using Internet Explorer, and what version (I've only tested it on 7.0) ?
Also did you assign the weights such that CKEditor_GeSHi (Filter) is BEFORE the GeSHi module's (Filter) - per installation instructions?
Finally, what type of code are you submitting within the form? If you can't see the code within CKEditor, then something is messing up in the Javascript plugin.

I'm working to add some additional error checking for form validation - though I cannot seem to replicate the issue you describe on my setup.
Changed the status to Needs Work, so that I can look into this and resolve it.

Thanks again :)

@ e-anima,
Thank you for the support and trial run. es ist sehr geschatzt :)