-
Notifications
You must be signed in to change notification settings - Fork 28
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
Parameter and BBCode simple values with syntax tokens #66
base: master
Are you sure you want to change the base?
Conversation
With
|
@@ -258,6 +258,28 @@ private function lookahead($type) | |||
return $this->position < $this->tokensCount && (empty($type) || $this->tokens[$this->position][0] === $type); | |||
} | |||
|
|||
private function lookaheadN(array $types) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't lookaheadN()
a more general way for lookahead()
? If yes, I would probably rewrite lookahead
like this:
private function lookahead($type)
{
return $this->lookaheadN([$type]);
}
I found a counter-example:
As you can see, the content is null because the ending |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Those changes don't parse [url= http://example.com/]link[/url]
correctly.
Alternatively, is it possible to have a parser that ignore self-closing tags like this |
@MrPetovan I'm sorry but I won't be able to continue work on this PR for now. You can try disabling self-closing tags by removing the part handling them in |
No problem, thanks for letting me know. Do you still plan on maintaining the project? |
@MrPetovan Yes, this project is very stable and I'm committed to maintaining it. It's just a temporary state where I won't have any time for side projects right now. I'm still thinking about your issue, though. It is clearly different from my closing-mark-always-closes-shortcode approach. Such cases are the very reason I introduced parameter delimiters |
Unfortunately I can’t expect Friendica users to start using delimiters to get the same end result as before we started to use your library. Backward compatibility with the current parser is a blocker, no matter how janky it currently is. |
I think arguments with spaces is separate issue, and this one is critical for my project. Surprised it wasn't yet merged in. #65 is pretty much fixed by this PR. |
590cbfb
to
f1660ae
Compare
Hi @MrPetovan and @Fedcomp, I'd like to revive this PR as I think I've found a way to make both sides happy. I implemented a WIP commit here. The idea behind this commit is that there will be two I would be very grateful if you could test these changes and provide some more test cases so that I can polish the code and prepare final PR. By the way, please look at #72 which brings ~20x performance gain to |
Hi @thunderer and thanks for keeping at it! I'll try to see what I can test over the next week. |
@MrPetovan I just tagged |
Not yet, I didn't take the time for it. We are in a release candidate period which means that we're focusing on fixing bugs rather than introducing new features. But as soon as we release the next version, I'll be able to focus my time on this. It's been a long-standing issue and I'm eager to finally put it to rest. |
9351dfa
to
63cafc5
Compare
I won't have the time to test this but it shouldn't stop you from merging. Thank you for your work! |
@MrPetovan what is the current status of shortcodes in Friendica? Will you be able to use the library now? |
Thanks for asking, not in the immediate future, but it still is planned, and this delay has nothing to do with the quality of this library. |
Issue #65.