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

Jump to bottom

Shortcodes #2193

Merged
merged 20 commits into from Dec 23, 2015
Merged

Shortcodes #2193

merged 20 commits into from Dec 23, 2015

Conversation

ralsina
Copy link
Member

@ralsina ralsina commented Dec 23, 2015

Implement hugo-style shortcodes to help markups reach feature-parity with RST.

@ralsina
* Added Hugo-like shortcode implementation
653fb9f 
* Registered post-list as a shortcode
* Added shortcode support to markdown compiler
@ralsina
updated changelog
8719341
@ralsina
lint
eaf1d30
Kwpolska
Kwpolska reviewed Dec 23, 2015
View reviewed changes
nikola/shortcodes.py
[['foo', (['bar'], {'bat': 'baz', 'data': 'some data'}), 0, 43]]
"""

# FIXME: this is really space-intolerant
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

y u no regex

@gour gour mentioned this pull request Dec 23, 2015
Closed
11 tasks
@Kwpolska
Fix docstrings
0543e7a 
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
Kwpolska
Kwpolska reviewed Dec 23, 2015
View reviewed changes
nikola/shortcodes.py

class SCParser(HTMLParser):
"""Parser for shortcode arguments."""
# Because shortcode attributes are HTML-like, we are abusing the HTML parser.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is overkill IMO. Mind me replacing with a really simple Python parser?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not mind, but after this branch is merged?

Kwpolska
Kwpolska reviewed Dec 23, 2015
View reviewed changes
docs/extending.txt

{{% foo %}} # No arguments
{{% foo bar %}} # One argument, containing "bar"
{{% foo bar baz=bat %}} # Two arguments, one containing "bar", one called "baz" containing "bat"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could use a note about "quotes" being accepted

@ralsina ralsina changed the title Shortcodes (WIP) Shortcodes Dec 23, 2015
@ralsina ralsina changed the title Shortcodes Shortcodes [WIP] Dec 23, 2015
@ralsina
Copy link
Member Author

ralsina commented Dec 23, 2015

What do you say @Kwpolska ? Merge?

@ralsina ralsina changed the title Shortcodes [WIP] Shortcodes Dec 23, 2015
@Kwpolska
Add basic shortcode tests
bdbb9a8 
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska
Copy link
Member

I would merge it, but I think that data should be a guaranteed argument (to make things easier for devs) — am I thinking right?

@ralsina
Copy link
Member Author

ralsina commented Dec 23, 2015

Why not? Done in the next revison.

@Kwpolska
Fix unused tests [ci skip]
79ab0a1 
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska
Adjust tests for data always existing
beb6b87 
Signed-off-by: Chris Warrick <kwpolska@gmail.com>
@Kwpolska
Copy link
Member

Merge if the newest commit passes tests.

Also, here’s a reason to drop HTMLParser:

{{% test "hello world" %}}
→ ('"hello', 'world"')

(I’ll send a PR for my parser after this gets merged)

@ralsina
Copy link
Member Author

ralsina commented Dec 23, 2015

Well, AFAIK that's not valid "html-like", is it? But sure, looking forward to the parser :-)

ralsina added a commit that referenced this pull request Dec 23, 2015
@ralsina
Merge pull request #2193 from getnikola/shortcodes
19cd8dd 
Shortcodes
@ralsina ralsina merged commit 19cd8dd into master Dec 23, 2015
@ralsina ralsina deleted the shortcodes branch December 23, 2015 20:39
@felixfontein
Copy link
Contributor

Out of curiosity: how is {{% test "%}}" %}} handled?

@Kwpolska
Copy link
Member 

nikola.shortcodes.apply_shortcodes('hello {{% test "%}}" %}} world', {'test': lambda a, **kw: ':' + a + ':'}, None)
'hello ::" %}} world'

@felixfontein
Copy link
Contributor

Hmm, that's not what I would expect as an end-user (with programming experience). Is this really intended? If it is, shouldn't the parser complain about the not well-formed statement?

@Kwpolska
Copy link
Member

Please write a better parser then.

@felixfontein
Copy link
Contributor

It has to be a combined parser for the regex part and the argument splitting, then. I'll work on that later.

@felixfontein
Copy link
Contributor

There are some more problems with the existing parsers:

from nikola import shortcodes

#print(shortcodes.apply_shortcodes('==> {{% <==', {}))
#print(shortcodes.apply_shortcodes('==> {{% a %%} <==', {}))
#print(shortcodes.apply_shortcodes('==> {{% a %}} {{% /a %%} <==', {}))
#print(shortcodes.apply_shortcodes('==> {{% test %}} {{% bla %}} {{% /bla %}} {{% /test %%} <==', {}))
#print(shortcodes.apply_shortcodes('==> {{% test %}} {{% bla %}} {{% /bla %}} {{% /bla %%} <==', {}))

Uncomment any of the lines to get an infinite loop.

@felixfontein
Copy link
Contributor

See #2200 for a new parser.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@ralsina @Kwpolska @felixfontein