Please provide working examples

NishantSolanki commented 9 years ago

Your documentations seems to be hard to understand, Can you provide working examples for each of the base functions so that one can get better understanding?

Thanks

thunderer commented 9 years ago

@NishantSolanki Hi, thanks for reporting the issue! Of course I can provide (better) examples, but could you please tell me what is hard to understand for you? I'd like to understand where did I get wrong in my documentation so that I can avoid that in the future.

NishantSolanki commented 9 years ago

@thunderer Thanks so much for your quick reply, But I dont even know where to start, I installed the library using composer, and below is what I was trying.

<?php
require_once 'vendor/autoload.php';
use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\Shortcode;
use Thunder\Shortcode\Extractor\RegexExtractor;
use Thunder\Shortcode\Parser\RegexParser;
use Thunder\Shortcode\Processor\Processor;
//use Thunder\Shortcode\Shortcode\Shortcode;
use Thunder\Shortcode\Serializer\JsonSerializer;
$facade = ShortcodeFacade::create();
\\print_r($facade);
?>

<html>
    <head>
        <title>
            Modularization
        </title>
    </head>
    <body>
        [code]testing[/code]
    </body>
</html>

But obviously it won't work, I am missing something. But dont know where to start with.

Regards

thunderer commented 9 years ago

@NishantSolanki I'm now working on a Pull Request introducing regular parser in #20 and then I'll work on the promised documentation, but I wanted to help you with the example you provided above. It's written to use all the new code I wrote on master branch that will be marked as 1.0.0 release soon. Could you please tell me if it's good and easy to understand now?

<?php
// Load Composer autoloader manually, you won't need that if you use any
// framework or you have loaded it before:
require_once 'vendor/autoload.php';

// Just a couple of uses to simplify the code:
use Thunder\Shortcode\Parser\RegexParser;
use Thunder\Shortcode\Processor\Processor;
use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\ShortcodeInterface;
use Thunder\Shortcode\HandlerContainer\HandlerContainer;
use Thunder\Shortcode\Syntax\CommonSyntax;

// This is a text containing shortcodes you want to replace:
$text = '<html>
    <head>
        <title>Modularization</title>
    </head>
    <body>
        [code]testing[/code]
        [code]different[/code]
    </body>
</html>';

// Here you create a handler container in which you will tell processor how do
// you want to replace the shortcodes. Say that you want to only display its
// name and content:
$handlers = new HandlerContainer();
$handlers->add('code', function(ShortcodeInterface $shortcode) {
    return $shortcode->getName().': '.$shortcode->getContent();
});

// Here you have an example how to use facade, give it collection of handlers
// and syntax and you're good to go:
$facade = ShortcodeFacade::create($handlers, new CommonSyntax());
echo 'This was processed by facade: '."\n".$facade->process($text)."\n\n";

// Here you have an example how to use processor manually, give it a parser
// (which also needs a syntax) and collection of handlers and print the result:
$processor = new Processor(new RegexParser(new CommonSyntax()), $handlers);
echo 'This was processed by processor: '."\n".$processor->process($text)."\n\n";

NishantSolanki commented 9 years ago

I just used your above code, but gettning below error.

Fatal error: Class 'Thunder\Shortcode\HandlerContainer\HandlerContainer' not found in D:\wamp\www\modular\index.php on line 28

I checked and HandlerContainer class is not present in the library.

I think if we install it using composer than it returns different files, I manually downloaded HandlerContainer class from git and put it in the respective folder than I got below error

Catchable fatal error: Argument 1 passed to Thunder\Shortcode\ShortcodeFacade::create() must be an instance of Thunder\Shortcode\Syntax, instance of Thunder\Shortcode\HandlerContainer\HandlerContainer given, called in D:\wamp\www\modular\index.php on line 35 and defined in D:\wamp\www\modular\vendor\thunderer\shortcode\src\ShortcodeFacade.php on line 37

thunderer commented 9 years ago

@NishantSolanki From the code you posted previously I think you're using version v0.4.0. Can you update it to dev-master (it will be released as 1.0.0 soon)? I changed many things and finally reached stable interface there. Nevertheless, for that older version the code should be:

// reuse top part of previous comment, just add use for RegexExtractor

// In version 0.4.0 and previous you added shortcode handlers directly to
// facade and processor. In current dev-master that was extracted to
// HandlerContainer and you add it there. So the examples above should look
// like code below for version 0.4.0:

// Here you have an example how to use facade, null means default syntax, and
// array with handlers is given directly:
$facade = ShortcodeFacade::create(null, array(
    'code' => function(ShortcodeInterface $shortcode) {
        return $shortcode->getName().': '.$shortcode->getContent();
    },
));
echo 'This was processed by facade: '."\n".$facade->process($text)."\n\n";

// Here you have an example how to use processor manually, you need extractor
// (it was merged with parser in current dev-master, no more extractors) and
// parser. Shortcode handlers are registered using addHandler() method:
$processor = new Processor(new RegexExtractor(), new RegexParser());
$processor->addHandler('code', function(ShortcodeInterface $shortcode) {
    return $shortcode->getName().': '.$shortcode->getContent();
});
echo 'This was processed by processor: '."\n".$processor->process($text)."\n\n";

NishantSolanki commented 9 years ago

Thanks for the quick one, I will try it as well One more thing I wanted to report, Just now I tried to install it using composer.json file

{
    "require": {
        "thunderer/shortcode": "~1.0"
    }
}

But composer gives an error The requested package "thunderer/shortcode" could not be found in any version, there may be a typo in package name

jdreesen commented 9 years ago

Use dev-master instead of ~1.0 because version 1.0 is not released yet.

thunderer commented 9 years ago

@NishantSolanki @jdreesen Yeah, that's because as you see on Shortcode's Packagist page there is no 1.0 version right now, and when you write ~1.0 in composer.json then it means "at least 1.0.0, but try to match as high 1.0.* as possible. The current highest stable release is 0.4.0. Apart from using dev-master you can also use @dev version.

NishantSolanki commented 9 years ago

Thanks 'dev-master' also gave the same error but '@dev' worked.

On Wed, Oct 14, 2015 at 11:17 AM, Tomasz Kowalczyk <notifications@github.com

wrote:

@NishantSolanki https://github.com/NishantSolanki @jdreesen https://github.com/jdreesen Yeah, that's because as you see on Shortcode's Packagist page there is no 1.0 version right now, and when you write ~1.0 in composer.json then it means "at least 1.0.0, but try to match as high 1.0.* as possible. The current highest stable release is 0.4.0. Apart from using dev-master you can also use @dev version.

— Reply to this email directly or view it on GitHub https://github.com/thunderer/Shortcode/issues/19#issuecomment-147942245.

Regards Nishant Solanki

thunderer commented 9 years ago

@NishantSolanki Weird, I tested both and they worked. Can you share how did you install this library? I ran in command line both:

composer require thunderer/shortcode=dev-master
composer require thunderer/shortcode=@dev

and they worked fine. These two are equivalent to putting requirement on versions dev-master and @dev in composer.json.

NishantSolanki commented 9 years ago

no.. I was doing it using composer.json file first I tried this

{
    "require": {
        "thunderer/shortcode": "dev-master"
    }
}

but that didnt worked and than

{
    "require": {
        "thunderer/shortcode": "@dev"
    }
}

Worked :)

On Wed, Oct 14, 2015 at 2:38 PM, Tomasz Kowalczyk notifications@github.com wrote:

@NishantSolanki https://github.com/NishantSolanki Weird, I tested both and they worked. Can you share how did you install this library? I ran in command line both:

composer require thunderer/shortcode=dev-master composer require thunderer/shortcode=@dev

and they worked fine. These two are equivalent to putting requirement on versions dev-master and @dev in composer.json.

— Reply to this email directly or view it on GitHub https://github.com/thunderer/Shortcode/issues/19#issuecomment-147984332.

Regards Nishant Solanki

thunderer commented 9 years ago

@NishantSolanki I just created a fresh composer.json and added this package and both composer install and composer update worked fine. The code is below, I'm unsure what could have gone wrong in your case. Have you updated composer? What's your PHP version? If it's working then okay, I'll tag 1.0.0 soon so we'll forget about the issue. :)

{
    "name": "test/test",
    "require": {
      "thunderer/shortcode": "dev-master"
    }
}

robehickman commented 8 years ago

Your examples for this project would be a lot more useful if they included the needed name-space use statements, as they are missing just copying these examples they do not work without messing around to figure out what name spaces need to be included.

Is it really necessary to use quite so many name-spaces? It would be much easier to deal with if the whole library was under a single, or possibly a few, of them.

robehickman commented 8 years ago

I ended up solving my problem using wordpress' parser.

thunderer commented 8 years ago

Hi @robehickman, I'm glad that WordpressParser worked for you, but please tell me what problem did you have? As for your questions:

including namespaces in examples is definitely a good idea. I didn't have enough spare time to finish and publish the documentation I'm working on since the last comments, but that's already fixed there,
putting different library parts under separate namespaces is good because it reduces time to search for the thing you want, it also lowers the cognitive load to remember where everything is (not to mention easier development). If you were to put everything in the single namespace, you'll have dozens of classes without any distinction and you'll really hate it in the long run. :) I think few more use statements is a really low cost for all the benefits it gives.

I think I explained my position, but maybe you have better ideas - let me know!

robehickman commented 8 years ago

I assume point #2 is with reference to an IDE enumerating the classes, I do not use one as I prefer Vim, and without having a tool to automate this having so many name-spaces is a pain.

robehickman commented 8 years ago

I have never had any issue whatsoever with having everything in a single namespace, as I use file-system structure, auto-loading, and naming conventions to provide structure in my code.

thunderer commented 8 years ago

@robehickman I found a plugin for Vim that might be useful for you, but yes, good IDE does that automatically and I really recommend trying out PHPStorm. I understand that if you aren't willing to use IDE that that may be a pain - I've been there and made decision to switch years ago. That's why I also would like PHP to have proper package system one day so that you could just say something like import thunder.shortcode.*; and have everything accessible without specific uses.

robehickman commented 8 years ago

I like the keyboard mappings that Vim has, and strongly dislike typical 'word processor like' text editors for code editing. Does PHPStorm have a vim emulator?

thunderer commented 8 years ago

@robehickman yes, have a look at IdeaVim.

robehickman commented 8 years ago

Thanks.

Considering php's dynamic features it wouldn’t surprise me if it is possible to implement something equivalent to import * as a function. I already have helpers so I can say 'instance_model('name')' which imports if it isn't already, instances the class and returns it. Because of these helpers I practically never include anything manually, which I find makes the code much less verbose.

thunderer commented 8 years ago

@NishantSolanki I had some time to finish the new README and current state can be seen in PR #34. Could you please read it and tell me whether it is okay for you now? Any suggestions on what to improve will be appreciated.

@robehickman Sorry for not getting back to you. As for your instance_model('name') approach I must say that this is against software engineering principles as such implementation directly hinders composability of your code. Also since loading of certain classes is dependent on your code it can be a problem when trying to integrate 3rd party solutions. When you use regular classes then you have all the power of autoloader and you can compose them whichever way you want. Using this library as an example you can build a class like ShortcodeFacade that encapsulates all the features related to shortcodes that you need in the project and for that you don't need to know anything apart from how to instantiate them and what is their public interface. A couple of uses is a negligible cost given how much freedom you have while writing code. What do you think?

NishantSolanki commented 8 years ago

@thunderer the README file looking great, you have done a quite amount of work.

But I didn't get that why you used that assert() function in your example, may be something like below can be a good quick start for beginners. :)

$handlers = new HandlerContainer();

$handlers->add('profile', function(ShortcodeInterface $s)
{
    return "Hello my name is " . $s->getParameter('name') . ", and my age is " . $s->getParameter('age');
});

$processor = new Processor(new RegularParser(), $handlers);
$string = '<div>[profile name="Thomas" age="24"] <br/> Some other text without the short code</div>';
echo $processor->process($string);

thunderer commented 8 years ago

@NishantSolanki I get the idea, but do you think that those asserts are too hard to understand? I tried to provide "real world" examples while writing it, but I felt that they were too verbose. :) I'll try to do something about it, but I'm happy that you're satisfied.

rhukster commented 8 years ago

I agree it does take some time to decode the tests. Understanding would come much quicker with some more real-world examples.

thunderer commented 8 years ago

@NishantSolanki @rhukster @robehickman PR #34 with new README was merged, I changed the main example to be more "real world" just as you have suggested. I'll think about other ones (and possibly setting up a Wiki) in the future, but this is not blocking right now.

@NishantSolanki could you please close this issue if the current state is good enough for you?

NishantSolanki commented 8 years ago

@thunderer that looks great, thanks for accepting my suggestions :+1:

thunderer / Shortcode

Please provide working examples #19