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

do not force the permalink to be a dir if it ends on .html #963

Merged
merged 2 commits into from Aug 11, 2013

Conversation

Projects
None yet
6 participants
@maul-esel
Contributor

maul-esel commented Apr 13, 2013

Fixes #798.

The rake tests seem to pass. Still, this modifies current behaviour and is slightly backwards incompatible. However, would you specify a permalink ending in ".html" if you wanted it to be a directory? (And if you used the specified permalink for linking, i.e. without the index.html part, it will still work.)

@mattr-

This comment has been minimized.

Member

mattr- commented Apr 14, 2013

Took me several times of looking at it to figure how it solved the original issue.

The fact that the tests don't fail bothers me though. I would be appreciated if you could write a test that makes sure this explicit case is covered, unless you can point out an existing test that verifies the behavior here.

test per-post permalinks
Add tests for setting a permalink on a post (in YAML frontmatter),
both the "old way" (generating an index.html) and with an .html ex-
tension.
@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Apr 14, 2013

It seems to me that so far there are no cucumber tests at all for setting the permalink in YAML. I did a "search in files" for "permalink" and only found references like this:

And I have a configuration file with "permalink" set to "pretty"

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented May 10, 2013

Any news on this?

@zachgersh

This comment has been minimized.

Contributor

zachgersh commented Jun 5, 2013

@parkr @mattr- probably a good thing to merge here?

@parkr

This comment has been minimized.

Member

parkr commented Jun 23, 2013

We definitely need to be ultra-careful of being backwards-compatible. If we break a bunch of sites, there's no telling the wrath of those we have offended and I'd rather keep wrath (and support emails to GitHub Support) low :) Do you think it's OK to close this?

@maul-esel

This comment has been minimized.

Contributor

maul-esel commented Jun 23, 2013

Your decision. I find the behaviour of appending /index.html quite annyoing - if you let the user choose, then do what he specifies, not something similar.

Just let's be clear about what breaks:

  • If I create a post
  • and set the permalink to whatever.html
  • and somewhere link to it as whatever.html/index.html (who would do that?),

then that link breaks. Or could you think of anything else that could break?

@kelvinst

This comment has been minimized.

kelvinst commented Jul 30, 2013

@parkr, sorry if you alrealdy discuss this in other issue or PR, but I have a question: does the GitHub team already thought in make a version control for Jekyll in GitHub Pages? Something like a configuration file for gh-pages read and use the right Jekyll version?

I feel that sometimes the dependency of GitHub pages are blocking some new implementations that would be very nice to have. And about versioning and deprecations, my thought is that maintain the backwards compatibility in patch versions are necessary, in minor versions are recommended and in major versions are optional. Then, we can put this behavior breaker PRs in a major version and plan it for a Jekyll 2.0.

What do you think about?

@parkr

This comment has been minimized.

Member

parkr commented Aug 8, 2013

@kelvinst That's a good idea, but it's quite the undertaking for the Pages team and presents yet another point of failure. We want to allow for breaking of some things, but I suspect the thousands of people using Jekyll by proxy would find that really frustrating. We need to maintain backwards compatibility for all releases until 2.0. Maybe by the time 2.0 comes around, we'll have figured out a better way to deal with backwards-compatibility.

@parkr parkr merged commit c0dfe31 into jekyll:master Aug 11, 2013

1 check passed

default The Travis build passed
Details

parkr added a commit that referenced this pull request Aug 11, 2013

@parkr

This comment has been minimized.

Member

parkr commented Aug 11, 2013

The merge couldn't be performed automatically so I've done it manually. Thanks!

@maul-esel maul-esel deleted the maul-esel:permalink-no-dir branch Aug 17, 2013

@jekyll jekyll locked and limited conversation to collaborators Feb 27, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.