Name: Markdown Insert
Core: 7.x
Project Page: https://drupal.org/sandbox/skh/2271863
Git: git clone --branch 7.x-1.x http://git.drupal.org/sandbox/skh/2271863.git markdown_insert
Review:http://pareview.sh/pareview/httpgitdrupalorgsandboxskh2271863git
Description
The Insert module outputs HTML, but when using Markdown syntax (text format), it is preferable to have this output in Markdown for consistency.
This module alters the output of the Insert module to use Markdown syntax for image_image
and file_generic
widget types.
Reviews
https://drupal.org/node/2278193#comment-8836857
https://drupal.org/node/2284061#comment-8867541
https://www.drupal.org/node/2298389#comment-8946001
Comments
Comment #1
skh CreditAttribution: skh commentedComment #2
lonaloreHi skh,
Your module seems to be an error and did not passed pareview test see this link:
http://pareview.sh/pareview/httpgitdrupalorgsandboxskh2271863git
And I noticed that, there is a typo in your .install file. The single quote mark is after LIMIT 1, and not after markdown_insert.
Comment #3
skh CreditAttribution: skh commentedThank you for the review. I have updated as per your remarks, and re-ran the tests at http://pareview.sh/pareview/httpgitdrupalorgsandboxskh2271863git
Comment #4
skh CreditAttribution: skh commentedGit branch name changed
Comment #5
dimple_chandra CreditAttribution: dimple_chandra commentedPlease add README.txt
Kindly mention in the description what is different in this module?
Comment #6
skh CreditAttribution: skh commented@dimple_chandra: Thanks for the review. However, there already is a README.txt, and I think the description states rather clearly it's intended purpose. Please let me know if there is something unclear. The module quite literally just alters the insert module to output markdown syntax instead of HTML.
Comment #7
dimple_chandra CreditAttribution: dimple_chandra commentedI cloned the module but i could not find README.txt
Comment #8
skh CreditAttribution: skh commentedhttp://cgit.drupalcode.org/sandbox-skh-2271863/tree/README.txt
Seems to be there? Not sure why it isn't there for you, but if you believe it's something on my end please let me know!
Thanks.
Comment #9
PA robot CreditAttribution: PA robot commentedWe are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)
Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).
I'm a robot and this is an automated message from Project Applications Scraper.
Comment #10
dimple_chandra CreditAttribution: dimple_chandra commentedThanks @skh i found the README.txt in your tree link but its strange while cloning i can't see that .txt
Comment #11
keopxUse README.txt official template https://drupal.org/node/2181737, because very difficult read it.
Comment #12
skh CreditAttribution: skh commentedOkay. I think that's rather subjective, but I have moved that readme to a .md file (in the hopes of 1674976), and added a txt.
Comment #13
skh CreditAttribution: skh commentedComment #14
skh CreditAttribution: skh commentedNeeds review state for 15 days as per 539608.
Comment #15
skh CreditAttribution: skh commentedAdded review.
Comment #16
keopx@skh read about Review bonus to have more visibility.
Comment #17
skh CreditAttribution: skh commentedComment #18
skh CreditAttribution: skh commentedComment #19
keopxHi change this format, example: Implements of hook_form_FORM_ID_alter(). to
Implements hook_form_FORM_ID_alter(). Without of
You have errors on some files, see: http://pareview.sh/pareview/httpgitdrupalorgsandboxskh2271863git
Comment #20
er.pushpinderrana CreditAttribution: er.pushpinderrana commented@skh, your module looks good and I also installed the same on my machine. I also reviewed code and get confused with following query.
I am not sure why are you using LIMIT here because you have already added where condition with name then I think there is no need to use limit. It is just increasing extra overhead on query and need to be fixed.
Rest looks fine.
Thanks!
Comment #21
skh CreditAttribution: skh commentedThanks, both notes updated.
http://pareview.sh/pareview/httpgitdrupalorgsandboxskh2271863git
http://cgit.drupalcode.org/sandbox-skh-2271863/commit/?id=9f2085f0070d65...
Comment #22
klausiReview of the 7.x-1.x branch:
This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.
manual review:
Although you should definitely fix those issues they are not absolute critical application blockers, so I guess this is RTBC otherwise. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.
Assigning to dman as he might have time to take a final look at this.
Comment #23
skh CreditAttribution: skh commentedThank-you for the review. I have completed the revisions as suggested, with one note:
I did it this way because the insert module creates the hook, and the implementation is not found otherwise. They also seemed to do it the same way.
I also noticed a few other popular modules like views, admin_menu, and devel did the same thing.
I was thinking of just updating it to module_set_weight when creating a D8 version (like devel). For now, I've changed it to use db_update.
Comment #24
dman CreditAttribution: dman commentedUser testing:
Pretty good project page. Setup was straightforward, the setting was where it said it would be.
I enabled the modules, and added a 'markdown' text format with the markdown filter enabled. No wysiwyg of course, if we are choosing markdown.
Configured the field settings:
Enable insert button
Enable Markdown Insert
Chose 'link to file, and 'medium' as the default.
Added a page with an image to add. Pressed the button and It inserted a fugly blob:
... this ugliness is why I have given up on markdown and tokenizers when HTML would do better... *sigh*
But - it's exactly what I expected from this sort of module. So does it work?
Well, I *do* get the img rendered, but I get a load of crud around it also. Is that supposed to be happening?
If I disable the markdown_insert and just use normal (HTML) insert, it's all fine. If I re-enable it again, I get crud.
... the crud goes away when I disable 'link to file' option from insert module.
So, that's a bug, but one that can be fixed as a project issue.
I was also able to trivially break it by inserting "dodgy ] text [ into th' description" - so you should probably sanitize that better.
I would have maybe approached either the insert.module team to see if this option could be added as just a patch, but as it's a 'glue' module, I guess it's OK stand-alone.
Anyway, code looks fine and readable. it does what it should, using the right tools. Plenty of reviews above have been done and passed.
This can be green-lighted. With a few small things to look at as new issues for you.
------------------
Thanks for your contribution, skh!
I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.
Here are some recommended readings to help with excellent maintainership:
You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!
Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
Thanks to the dedicated reviewer(s) as well.
Comment #25
skh CreditAttribution: skh commentedThank-you to dman and everyone else for the great suggestions and help.
dman, you have inspired me to make markdown in Drupal more accessible and user-friendly. I have a couple of other markdown tools in development, and now have plans to add an option for reference-style links and potentially a simple WYSIWYG-style formatting toolbar.
Thanks again!
Sam.