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

Make sure that all twig files end up as .html files. #202

Merged
merged 1 commit into from Mar 14, 2015

Conversation

naxoc
Copy link
Contributor

@naxoc naxoc commented Feb 22, 2015

A simple - maybe too simple - solution to the problem in #172

@simensen
Copy link
Member

simensen commented Mar 4, 2015

@naxoc FWIW, I haven't forgotten about this. I thought I had responded at least, though. My bad.

I think this solution may indeed be too simple. I think it might work but I think I'd rather have this happening elsewhere. I need to think on it some. :)

@ibrasho
Copy link

ibrasho commented Mar 5, 2015

I think this should be implemented in Sculpin\Core\Permalink\SourcePermalinkFactory. Most probably in SourcePermalinkFactory::generatePermalinkPathname().

Just ensure that the filename extension is .html.

@simensen What do you think?

@simensen
Copy link
Member

simensen commented Mar 5, 2015

@ibrasho That sounds reasonable. It is closer to what I had in mind. Probably right after $pathname = $source->relativePathname(); is called, we could run this code to remove the .twig bit from the pathname. I'm still not super happy about this, but that is mostly because SourcePermalinkFactory is a bad implementation anyway. :)

@naxoc How do you feel about moving this around to where @ibrasho suggested?

@naxoc
Copy link
Contributor Author

naxoc commented Mar 6, 2015

That sounds good to me. I'll give it a go in the weekend :)

@naxoc
Copy link
Contributor Author

naxoc commented Mar 7, 2015

Still super simple - but in a place it makes much more sense now.

simensen added a commit that referenced this pull request Mar 14, 2015
Make sure that all twig files end up as .html files.
@simensen simensen merged commit 72334e1 into sculpin:master Mar 14, 2015
@simensen
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

None yet

3 participants