Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nested shortcode issue #57

Closed
jenstornell opened this issue Jan 26, 2018 · 15 comments
Closed

Nested shortcode issue #57

jenstornell opened this issue Jan 26, 2018 · 15 comments
Assignees
Labels

Comments

@jenstornell
Copy link

jenstornell commented Jan 26, 2018

Fails

<?php
include 'vendor/autoload.php';

use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\ShortcodeInterface;

$facade = new ShortcodeFacade();
$facade->addHandler('hello', function(ShortcodeInterface $s) {
    return sprintf('Hello, %s!' . $s->getContent(), $s->getParameter('name'));
});

$text = '
    <p>Start</p>
    [hello name="Thomas"]
        [hello name="Peter"]
    [/hello]
    <p>End</p>
';
echo $facade->process($text);

Result

<p>Start</p>
Hello, Thomas!
  [hello name="Peter"]
[/hello]
<p>End</p>

Works

$text = '
    [hello name="Thomas"]
        [hello name="Peter"]
    [/hello]
';

Result

Hello, Thomas!
  Hello, Peter!
@thunderer
Copy link
Owner

Hi @jenstornell, I also ran your script here on the same PHP versions with the same setup as in #56, with the same result: correct, both shortcodes replaced as they should.

BTW I assume that "Logan" in the first result is a typo because I'm pretty sure I don't detect and replace names in the source text. :)

@jenstornell
Copy link
Author

@thunderer Yes, "Logan" was a typo.

As I already write I can tell about the backstory of using your library. I have an idea to write a plugin for Kirby that would be a replacement for their built in "Tags":

https://getkirby.com/docs/developer-guide/kirbytext/tags

And the reason for the replacement is mainly this issue:

getkirby/kirby#628

...but it would also be nice to have something more powerful with support for nesting etc.

You are sure you run exatcly this code when you tested?

<?php
include 'vendor/autoload.php';

use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\ShortcodeInterface;

$facade = new ShortcodeFacade();
$facade->addHandler('hello', function(ShortcodeInterface $s) {
    return sprintf('Hello, %s!' . $s->getContent(), $s->getParameter('name'));
});

$text = '
    <p>Start</p>
    [hello name="Thomas"]
        [hello name="Peter"]
    [/hello]
    <p>End</p>
';
echo $facade->process($text);

@thunderer
Copy link
Owner

Thanks for the information, @jenstornell. A couple more questions then:

  • do you have mbstring extension installed and enabled?
  • have you overwritten its functions?
  • are all Shortcode tests passing in your environment? Please share PHPUnit output.
  • have you tried the code in a Linux environment?

Also, yes, I'm sure that I ran exactly the code you mentioned, you can see the Gist with all my code to reproduce your issue: https://gist.github.com/thunderer/862b87b0cd26916387510ab81f2c418b .

@jenstornell
Copy link
Author

jenstornell commented Jan 26, 2018

Some server info is in the image below. Multibyte support is enabled. I have not overwritten it. It's a very standard XAMPP setup.

image

It seems complicated to install PHP Unit so I have been able to code decently without it so far. I can't test it because of limited skills.

I have not tried the code on Linux, but Apache is a Linux server, right?

It looks like you are running the code as it should.

@thunderer
Copy link
Owner

@jenstornell I see that mbstring seems to be enabled. To generate PHPUnit output please clone this repository in a separate directory, enter it, run composer install and then run php vendor/bin/phpunit and paste its output here. That's all.

Apache is an HTTP server that is indeed commonly run under Linux, but it's not a Linux server per se. You are using a version compiled for Windows. If you could create an Ubuntu VM in VirtualBox and check it, it would be very helpful to isolate the problem. But PHPUnit output is crucial here if some of the tests are failing under Windows (even though I also ran them there and they passed) then I need to know which.

@thunderer
Copy link
Owner

I managed to get fresh XAMPP with PHP 7.2 running on Windows 10 on my laptop, ran the tests, all green. apart from running Shortcode tests, could you "debug" the code using var_dump() to check what shortcodes are reported by parser? I'm really curious why it is not working with all the evidence I was able to gather so far.

@jenstornell
Copy link
Author

jenstornell commented Jan 26, 2018

I just get this:

D:\projects\shortcode-tests>php vendor/bin/phpunit
dir=$(cd "${0%[/\\]*}" > /dev/null; cd "../phpunit/phpunit" && pwd)
if [ -d /proc/cygdrive ] && [[ $(which php) == $(readlink -n /proc/cygdrive)/* ]]; then
   # We are in Cgywin using Windows php, so the path must be translated
   dir=$(cygpath -m "$dir");
fi
"${dir}/phpunit" "$@"

If I go directly to vendor/bin and then type phpunit, I get a list of things I can do.

Update

You already have run the tests so I get to the var_dump instead. This is the $s object:

object(Thunder\Shortcode\Shortcode\ProcessedShortcode)#16 (15) {
  ["parent":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  NULL
  ["position":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(1)
  ["namePosition":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(1)
  ["text":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  string(107) "
    <p>Start</p>
    [hello name="Thomas"]
        [hello name="Peter"]
    [/hello]
    <p>End</p>
"
  ["textContent":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  NULL
  ["offset":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(24)
  ["baseOffset":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(24)
  ["shortcodeText":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  string(21) "[hello name="Thomas"]"
  ["iterationNumber":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(1)
  ["recursionLevel":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  int(0)
  ["processor":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  object(Thunder\Shortcode\Processor\Processor)#8 (6) {
    ["handlers":"Thunder\Shortcode\Processor\Processor":private]=>
    object(Thunder\Shortcode\HandlerContainer\HandlerContainer)#4 (2) {
      ["handlers":"Thunder\Shortcode\HandlerContainer\HandlerContainer":private]=>
      array(1) {
        ["hello"]=>
        object(Closure)#13 (1) {
          ["parameter"]=>
          array(1) {
            ["$s"]=>
            string(10) "<required>"
          }
        }
      }
      ["default":"Thunder\Shortcode\HandlerContainer\HandlerContainer":private]=>
      NULL
    }
    ["parser":"Thunder\Shortcode\Processor\Processor":private]=>
    object(Thunder\Shortcode\Parser\RegularParser)#6 (5) {
      ["lexerRegex":"Thunder\Shortcode\Parser\RegularParser":private]=>
      string(113) "~((?<open>\[)|(?<close>\])|(?<marker>\/)|(?<separator>\=)|(?<delimiter>\")|(?<ws>\s+)|(?<string>[\w-]+|\\.|.))~us"
      ["tokens":"Thunder\Shortcode\Parser\RegularParser":private]=>
      array(44) {
        // REMOVED LIST OF TOKENS FOR READABILITY
      }
      ["tokensCount":"Thunder\Shortcode\Parser\RegularParser":private]=>
      int(44)
      ["position":"Thunder\Shortcode\Parser\RegularParser":private]=>
      int(44)
      ["backtracks":"Thunder\Shortcode\Parser\RegularParser":private]=>
      array(1) {
        [0]=>
        array(0) {
        }
      }
    }
    ["eventContainer":"Thunder\Shortcode\Processor\Processor":private]=>
    object(Thunder\Shortcode\EventContainer\EventContainer)#5 (1) {
      ["listeners":"Thunder\Shortcode\EventContainer\EventContainer":private]=>
      array(0) {
      }
    }
    ["recursionDepth":"Thunder\Shortcode\Processor\Processor":private]=>
    NULL
    ["maxIterations":"Thunder\Shortcode\Processor\Processor":private]=>
    int(1)
    ["autoProcessContent":"Thunder\Shortcode\Processor\Processor":private]=>
    bool(true)
  }
  ["name":protected]=>
  string(5) "hello"
  ["parameters":protected]=>
  array(1) {
    ["name"]=>
    string(6) "Thomas"
  }
  ["content":protected]=>
  NULL
  ["bbCode":protected]=>
  NULL
}

Output of it (same as before):

    <p>Start</p>
    Hello, Thomas!
        [hello name="Peter"]
    [/hello]
    <p>End</p>

@thunderer
Copy link
Owner

  ["shortcodeText":"Thunder\Shortcode\Shortcode\ProcessedShortcode":private]=>
  string(21) "[hello name="Thomas"]"

@jenstornell The first shortcode is correctly extracted from the text, but is there only one, not two? The second one should be [hello name="Peter"][/hello]. Can you create a RegularParser instance and paste the result of parsing this text? $parser = new RegularParser(); var_dump($parser->parse($text)); Also maybe we can work it out together, can I connect to your machine using TeamViewer and do some debugging myself? I'm very curious what is happening there as I've tried your code in a variety of environments that I have access to, with no luck of reproducing the problem.

Please run the tests by executing php vendor/phpunit/phpunit/phpunit, because vendor/bin/phpunit is just a script that launches PHPUnit. I assumed that the same command would work just like on Linux, turns out it won't.

@jenstornell
Copy link
Author

I tried it at home on my laptop with the same result. Here I also use Windows with XAMPP so the result is expected. I also installed it with Composer.

<?php
include 'vendor/autoload.php';

use Thunder\Shortcode\ShortcodeFacade;
use Thunder\Shortcode\Shortcode\ShortcodeInterface;
use Thunder\Shortcode\Parser\RegularParser;

$facade = new ShortcodeFacade();
$facade->addHandler('hello', function(ShortcodeInterface $s) {
    return sprintf('Hello, %s!' . $s->getContent(), $s->getParameter('name'));
});

$text = '
    <p>Start</p>
    [hello name="Thomas"]
        [hello name="Peter"]
    [/hello]
    <p>End</p>
';

$parser = new RegularParser();
var_dump($parser->parse($text));

echo $facade->process($text);

It output the var_dump and also the result of the echo.


array(1) {
  [0]=>
  object(Thunder\Shortcode\Shortcode\ParsedShortcode)#17 (6) {
    ["text":"Thunder\Shortcode\Shortcode\ParsedShortcode":private]=>
    string(21) "[hello name="Thomas"]"
    ["offset":"Thunder\Shortcode\Shortcode\ParsedShortcode":private]=>
    int(24)
    ["name":protected]=>
    string(5) "hello"
    ["parameters":protected]=>
    array(1) {
      ["name"]=>
      string(6) "Thomas"
    }
    ["content":protected]=>
    NULL
    ["bbCode":protected]=>
    NULL
  }
}

    <p>Start</p>
    Hello, Thomas!
        [hello name="Peter"]
    [/hello]
    <p>End</p>

I don't know if it help you. If not, maybe I can mail you a zip of my files. Then you can try a copy of my files, in case they are different in some way.

I'll get back to you with phpunit tests if I get it to run.

@jenstornell
Copy link
Author

Phpunit tests. A driver is missing but it seems to have run anyway. So all tests passed, I guess, at 276/276 100%.

PHPUnit 6.5.5 by Sebastian Bergmann and contributors.

Error:         No code coverage driver is available

...............................................................  63 / 276 ( 22%)
............................................................... 126 / 276 ( 45%)
............................................................... 189 / 276 ( 68%)
............................................................... 252 / 276 ( 91%)
........................                                        276 / 276 (100%)

Time: 628 ms, Memory: 6.00MB

OK (276 tests, 2058 assertions)

Also see my previous comment about the var_dump test.

@jenstornell
Copy link
Author

If you want to test exactly the same files as me, I created a new repo for you here:

https://github.com/jenstornell/shortcode-test

It returns the same thing as a comment above, var_dump of the parse and this output:

<p>Start</p>
    Hello, Thomas!
        [hello name="Peter"]
    [/hello]
<p>End</p>

@thunderer
Copy link
Owner

thunderer commented Jan 29, 2018

@jenstornell If that's exactly the code you're running then I know what happened - you're using an old version v0.6.0, not the latest v0.6.5. Run composer info to see what package versions you have installed, when I cloned your repository I saw:

$ composer info                             
thunderer/shortcode v0.6.0 Advanced shortcode (BBCode) parser and engine for PHP

which matches the "thunderer/shortcode": "0.6" entry in composer.json. If you upgrade to v0.6.5 through composer require thunderer/shortcode=^0.6 (note the caret in ^0.6) then your issue should be resolved.

@jenstornell
Copy link
Author

The problem is solved, but not the way you think. But you are correct, the version I have is 0.6.

Updates back to 0.6

So I run the update process:

D:\projects\shortcode-test>composer require thunderer/shortcode=^0.6

./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Nothing to install or update
Generating autoload files

I expected it to be 0.6.5 now, but nope:

D:\projects\shortcode-test>composer info

thunderer/shortcode v0.6.0 Advanced shortcode (BBCode) parser and engine for PHP

Updates to 0.6.5

D:\projects\shortcode-test>composer require thunderer/shortcode=^0.6.5

./composer.json has been updated
Loading composer repositories with package information
Updating dependencies (including require-dev)
Package operations: 0 installs, 1 update, 0 removals
  - Updating thunderer/shortcode (v0.6.0 => v0.6.5): Downloading (100%)
Writing lock file
Generating autoload files

Here I don't even need to check the version number as it says it in the process above. Anyway, composer require thunderer/shortcode=^0.6.5 worked but composer require thunderer/shortcode=^0.6 just installed version 0.6.

Maybe the issue is the command line tool? I use the built in cmd.exe (Windows Termial / command prompt).

@thunderer
Copy link
Owner

Interesting, when I ran the mentioned command with =^0.6 on your repository, it upgraded the library as expected. Maybe upgrade also your Composer? But I'm very happy that we've found the issue and the latest version is indeed working for you. Can you close this issue and also check whether #56 also works as expected? Thanks!

@thunderer thunderer self-assigned this Jan 29, 2018
@jenstornell
Copy link
Author

It's me who shall be thanking you for all the effort to solve this problem. Great thanks! =)

As a sidenote, I run Composer version 1.6.2 2018-01-05 15:28:41 the latest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants