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

Add the option to configure TEASER_REGEXP #1946

Merged

Conversation

@yamila-moreno
Copy link
Contributor

@yamila-moreno yamila-moreno commented Aug 15, 2015

For those who migrate from wordpress (thanks for the importer!), we have the "" tag for the "Read more" link. But the new posts will have ".. TEASER_END" as recommended.

So I'll end with two different "strings" to detect as TEASER_REGEXP.

For that reason, I suggest those changes:

  • by default the behaviour is the same
  • but if you need to use two (or more!!) different ways, you can create the regexp in the conf.py
@ralsina
Copy link
Member

@ralsina ralsina commented Aug 15, 2015

Changing this to a regexp makes me nervous :-)
We could make it configurable but a string, and you can just use "read more" ... would that be bad?

@yamila-moreno
Copy link
Contributor Author

@yamila-moreno yamila-moreno commented Aug 15, 2015

Hi! I tried to make as few changes as possible. But I can change it, or sure. If I understood this correctly, you want something like:

TEASER_ENDS = '|'.join(['TEASER_END', 'more'])

and use this within the regular expression. Is that what you meant?

Thanks for the interest!! :D

@ralsina
Copy link
Member

@ralsina ralsina commented Aug 15, 2015

Ah, no, I was just wrong :-)

Your change is totally ok, let's merge it!

Can you add yourself to AUTHORS.txt and write a one-liner for CHANGES.txt?

@yamila-moreno
Copy link
Contributor Author

@yamila-moreno yamila-moreno commented Aug 15, 2015

Ok! In a moment I'll push -f :D #yay

@yamila-moreno yamila-moreno force-pushed the yamila-moreno:ENHANCEMENT/teaser_regexp_customizable branch from ed4ce53 to fad190e Aug 15, 2015
@yamila-moreno
Copy link
Contributor Author

@yamila-moreno yamila-moreno commented Aug 15, 2015

Here you are the changes 👍

@ralsina
Copy link
Member

@ralsina ralsina commented Aug 15, 2015

Checking the code a bit more, you are setting TEASER_REGEXP in conf.py but the code in post.py will still use the one defined in post.py line 75.

You will also need to remove that line 75 and use self.config['TEASER_REGEXP'] instead of TEASER_REGEXP in Post.text() so you use the one you configured.

@yamila-moreno
Copy link
Contributor Author

@yamila-moreno yamila-moreno commented Aug 15, 2015

Not sure if I understand what you mean, sorry.

In nikola/post.py I've added:

teaser_regexp = self.config.get('TEASER_REGEXP', TEASER_REGEXP)

with this line, I choose the conf.py value (if exists) or set the default value (line 75). From this point minimun changes to use the new teaser_regexp variable.

Where is still used the original TEASER_REGEXP?

@ralsina
Copy link
Member

@ralsina ralsina commented Aug 15, 2015

You are right... not my best day to review code :-) Merging...

ralsina added a commit that referenced this pull request Aug 15, 2015
…_customizable

Add the option to configure TEASER_REGEXP
@ralsina ralsina merged commit 52f0b8c into getnikola:master Aug 15, 2015
2 checks passed
2 checks passed
continuous-integration/appveyor AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@yamila-moreno
Copy link
Contributor Author

@yamila-moreno yamila-moreno commented Aug 15, 2015

hahahaha, better to be sure!! 💃
Thanks for accepting the suggestion!

@ralsina
Copy link
Member

@ralsina ralsina commented Aug 15, 2015

You're welcome, and thank you for helping the project :-)

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

Successfully merging this pull request may close these issues.

None yet

2 participants