Skip to content

Need a namespaced library-style version #47

Closed
michelf opened this Issue Sep 30, 2012 · 10 comments

4 participants

@michelf
Owner
michelf commented Sep 30, 2012

Currently, the two versions available here (on branches master & extra) are meant to be many things to many people. It has manually-changeable configuration variables at the top, a function, a plugin interface for Wordpress, an emulation mode for Textile, and it is used all over the place in many different ways. The current layout of the file looks like this:

makdown.php:

@define(...) // configuration variables which can be overriden by putting a define prior including
@define(MARKDOWN_CLASS_NAME) // class name to use in Markdown function
function Markdown() {...} // markdown function
if ($wp_version) {...} // wordpress plugin
if ($plone) {...} // plone plugin interface
if (name of file is classTextile) {...} // textile compatibility interface
class Markdown_Parser {...} // markdown parser class
class MarkdownExtra_Parser {...} // markdown extra parser class (on extra branch)

Now, PHP 5.3 is no longer new and we still don't have autoloading support in PHP Markdown. Adding a namespace declaration at the top would break the many ways people have been using PHP Markdown up to now, so I can't just do that.

The discussion in #46 has convinced me it's time for a clean break. The old plugin-like version will stay, but I intend to create a new branch for a library-style PHP 5.3 namespace-aware version of PHP Markdown. Here's the proposed file layout:

michelf/Markdown/Parser.php

namespace michelf;
function Markdown() {...}
class Parser {...}

michelf/MarkdownExtra/Parser.php

namespace michelf;
function Markdown() {...}
class Parser {...}

Plain, clean, no plugin-thing crap, namespaced, and support for autoloading. This will also remove the strangeness of having the Markdown function calling a different parser depending on whether you downloaded PHP Markdown Extra or plain PHP Markdown.

Am I missing anything? Any use case not covered? Something that can be improved? Let me know.

@jnovack
jnovack commented Sep 30, 2012

There is already a fork which covers most of this and is currently working. @filp took your existing (v1.0.1n) code and restructured it as https://github.com/filp/Markdown

I have since added v1.0.1o features you've integrated and some you haven't from pull requests (most notably the HTML5 elements) as https://github.com/jnovack/Markdown (Note: I versioned it as 1.0.15, as o is the 15th letter of the alphabet and PEAR likes major.minor.revision numbers without letters).

I would not expect you to blindly accept this code, but it's already built, I and a few others already use it, and with your keen eye, this can be an easier project than you might have anticipated. Give it a look over, you might get ideas. Consider it crowd-sourced. ;)

@jnovack jnovack referenced this issue in filp/Markdown Oct 2, 2012
Merged

September Refresh #1

@michelf
Owner
michelf commented Oct 2, 2012

@jnovack Nice work, although, after thinking about it for a little while, it's hard for me to reuse it. For one thing, it's not properly forked from my Git repository, so any merging will need to be manual. For second, this only includes PHP Markdown and not the Extra parser; I intend to give the two the same treatment, and PHP SmartyPants too.

My goal right now is to make it easy to use as a PHP library, with auto loading, namespaces and stuff like that. While the requirement for this branch will be PHP 5.3, prettifying the code to match PHP 5 coding convention is less so, and this is because I still want the same code to be shared with the other branches which can work all the way back to PHP 4.

@Avalarion

Hi Michel,

just to understand you right, you want to create a branch for 5.3+ with the same code just the NameSpace and autoloading things so you're able to hold both at the same code base?

This would be the best thing you could do, and if you need any further help, just say a word!

Have a nice day,

Bastian

@jnovack
jnovack commented Oct 2, 2012

Thank you, but @filp did the hard work, I edited a few lines and spacing.

The code IS your code, its just put into classes and variables changed where needed. Its your structure and your flow. It wasn't a fork because it needed to be completely rewritten. If it was forked there would be very little lines in common if any.

I believe there is another fork on your network that is closer to what you may want. If you run through the open pull requests and forks you may find it before I do.

@michelf
Owner
michelf commented Oct 2, 2012

@Avalarion Yes, mostly. Making changes so it better fit 5.3 style is not out of question (such as using const instead of declare, and __construct instead of the class name), as long as git can merge those changes with minimal intervention on my part. But the short term goal is only to have a properly namespaced package.

@jnovack Yes, you're right. @flip did most of the work. I knew that but failed to acknowledge it.

@michelf
Owner
michelf commented Oct 2, 2012

I'm playing with this right now, and at the same time I'm rewriting the Usage section of the readme to match. Does this fit what you guys had in mind?

Usage

You can use PHP Markdown easily in your PHP program. This library package
is meant to be used with autoloading, so putting the 'michelf' folder
in your include path should be enough for this to work:

use \michelf\Markdown;
$my_html = Markdown::markdown($my_text);

PHP Markdown Extra is also available the same way:

use \michelf\MarkdownExtra;
$my_html = MarkdownExtra::markdown($my_text);

If you wish to use PHP Markdown with another text filter function
built to parse HTML, you should filter the text after the Markdown
function call. This is an example with PHP SmartyPants:

use \michelf\Markdown, \michelf\SmartyPants;
$my_html = Markdown::markdown($my_text);
$my_html = SmartyPants::smartypants(my_html);

All these examples are using the static markdown function found inside the
parser class. If you want to customize the parser, you can also instantiate
it directly and change some configuration variables:

use \michelf\MarkdownExtra;
$parser = new MarkdownExtra;
$parser->fn_id_prefix = "post22-";
$my_html = $parser->transform($my_text);
@cweiske
cweiske commented Oct 2, 2012

Markdown::markdown sounds like a PHP4 constructor. Why not Markdown::convert or ::toHtml()? This would also make the static method name consistent with MarkdownExtra and SmartyPants.

@michelf
Owner
michelf commented Oct 2, 2012

@cweiske Oops, forgot about that. That makes Markdown::markdown a bad idea indeed.

There's already a transform method, which is non-static, which is used both for PHP Markdown and PHP SmartyPants. I could use convert as the static one and transform as the non-static one, but I feel it'd be confusing. I guess I could hack it so that transform could be called statically or not, but I feel it'd go against the goal of cleaning things up.

toHTML() is also an option, but I'll have to find something else for SmartyPants, as both the input and the output is HTML for SmartyPants. Naming things is hard. I'll try to figure out something else.

@jnovack
jnovack commented Oct 2, 2012

gogoGadget? (90s American TV show reference for Inspector Gadget)

@michelf
Owner
michelf commented Oct 4, 2012

Thanks a lot for the feedback. See the new lib branch. Right now, it's pre-release. Eventually, this'll probably become the default branch.

@michelf michelf closed this Oct 4, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.