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 'jekyll new-theme' command to help users get up and running creating a theme #4848

Merged
merged 7 commits into from May 20, 2016

Conversation

Projects
None yet
5 participants
@parkr
Member

parkr commented Apr 27, 2016

This command allows users to create new themes very easily:

$ jekyll new-theme THEME_NAME
$ cd THEME_NAME
$ # add files
$ # commit files
$ bundle exec rake release

Just saving my place for now.

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr
Member

parkr commented Apr 27, 2016

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter Apr 27, 2016

Contributor

This may be a terrible idea, but what about jekyll new --theme? That way, down the line, jekyll new --theme [THEME] can create a new site with that theme, and jekyll new --theme can create a template site.

Alternatively, borrowing from rails g, jekyll new site (with site being implied when omitted) and jekyll new theme?

Contributor

benbalter commented Apr 27, 2016

This may be a terrible idea, but what about jekyll new --theme? That way, down the line, jekyll new --theme [THEME] can create a new site with that theme, and jekyll new --theme can create a template site.

Alternatively, borrowing from rails g, jekyll new site (with site being implied when omitted) and jekyll new theme?

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr Apr 27, 2016

Member

@benbalter the reason I don't want to nest it is that it makes it harder to find with just jekyll --help. In the spirit of bundle gem <name>, we could do jekyll theme <name>.

Member

parkr commented Apr 27, 2016

@benbalter the reason I don't want to nest it is that it makes it harder to find with just jekyll --help. In the spirit of bundle gem <name>, we could do jekyll theme <name>.

@envygeeks

This comment has been minimized.

Show comment
Hide comment
@envygeeks

envygeeks Apr 27, 2016

Contributor

I like @benbalter's jekyll new --theme too but I also agree with you @parkr, what about if jekyll new --theme just routed internally to jekyll new-theme because it would be intuitive for people to also try jekyll new --theme (I know I would try that first without even looking at the docs.)

Contributor

envygeeks commented Apr 27, 2016

I like @benbalter's jekyll new --theme too but I also agree with you @parkr, what about if jekyll new --theme just routed internally to jekyll new-theme because it would be intuitive for people to also try jekyll new --theme (I know I would try that first without even looking at the docs.)

@DirtyF

This comment has been minimized.

Show comment
Hide comment
@DirtyF

DirtyF Apr 27, 2016

Member

Do you think there could be more options related to theme management in the future?
If so, you may want to consider a dedicated theme command:

jekyll theme --new, -n THEME_NAME # create new theme
jekyll theme --clone, -c THEME_NAME # start a new theme from an existing one
jekyll theme --release, -r THEME_NAME # release a theme
jekyll theme --list, -ls # list all installed themes
jekyll theme --list-remote, lsr # list all remote themes
jekyll theme --search, -s # search for a THEME_NAME
jekyll theme --install, -i <url> # install a theme from a distant URL (gem or github repo)
jekyll theme --uninstall , -u THEME_NAME # uninstall a theme
Member

DirtyF commented Apr 27, 2016

Do you think there could be more options related to theme management in the future?
If so, you may want to consider a dedicated theme command:

jekyll theme --new, -n THEME_NAME # create new theme
jekyll theme --clone, -c THEME_NAME # start a new theme from an existing one
jekyll theme --release, -r THEME_NAME # release a theme
jekyll theme --list, -ls # list all installed themes
jekyll theme --list-remote, lsr # list all remote themes
jekyll theme --search, -s # search for a THEME_NAME
jekyll theme --install, -i <url> # install a theme from a distant URL (gem or github repo)
jekyll theme --uninstall , -u THEME_NAME # uninstall a theme

@parkr parkr referenced this pull request May 13, 2016

Closed

Release Jekyll v3.2 #4891

12 of 12 tasks complete
private
def root
@root ||= Pathname.new(File.expand_path("../", __dir__))

This comment has been minimized.

@envygeeks

envygeeks May 16, 2016

Contributor

@root ||= Pathutil.new("../").expand_path(__dir__) you should be able to do the same thing with Pathname.

@envygeeks

envygeeks May 16, 2016

Contributor

@root ||= Pathutil.new("../").expand_path(__dir__) you should be able to do the same thing with Pathname.

Show outdated Hide outdated lib/jekyll/theme_builder.rb
def initialize(theme_name)
@name = theme_name.to_s.gsub(/ /, "_").gsub(/_+/, "_")
@path = Pathname.new(File.expand_path(name, Dir.pwd))

This comment has been minimized.

@envygeeks

envygeeks May 16, 2016

Contributor

@path = Pathutil.pwd.join(name)

@envygeeks

envygeeks May 16, 2016

Contributor

@path = Pathutil.pwd.join(name)

Show outdated Hide outdated lib/jekyll/theme_builder.rb
Array(directories).each do |directory|
full_path = path.join(directory)
Jekyll.logger.info "create", "#{full_path}"
FileUtils.mkdir_p(full_path)

This comment has been minimized.

@envygeeks

envygeeks May 16, 2016

Contributor

If you convert @path to Pathutil you can simply do path.mkdir_p and it will do the rest for you.

@envygeeks

envygeeks May 16, 2016

Contributor

If you convert @path to Pathutil you can simply do path.mkdir_p and it will do the rest for you.

Show outdated Hide outdated lib/jekyll/theme_builder.rb
def write_file(filename, contents)
full_path = path.join(filename)
Jekyll.logger.info "create", "#{full_path}"
File.write(full_path, contents)

This comment has been minimized.

@envygeeks

envygeeks May 16, 2016

Contributor

This is a good candidate for Pathutil#write since it's Windows compatible and writes Windows compatible line endings on your behalf. This way Windows users get what they use natively and Linux/Unix will too.

@envygeeks

envygeeks May 16, 2016

Contributor

This is a good candidate for Pathutil#write since it's Windows compatible and writes Windows compatible line endings on your behalf. This way Windows users get what they use natively and Linux/Unix will too.

Show outdated Hide outdated lib/jekyll/theme_builder.rb
end
def create_accessories
%w{README.md CODE_OF_CONDUCT.md LICENSE.txt}.each do |filename|

This comment has been minimized.

@envygeeks

envygeeks May 16, 2016

Contributor

Why CODE_OF_CONDUCT? It's not our repository not our choice to decide if this should be in there.

@envygeeks

envygeeks May 16, 2016

Contributor

Why CODE_OF_CONDUCT? It's not our repository not our choice to decide if this should be in there.

parkr added some commits Apr 27, 2016

Show outdated Hide outdated lib/theme_template/README.md.erb
## License
The theme is available as open source under the terms of the [MIT License](http://opensource.org/licenses/MIT).

This comment has been minimized.

@parkr

parkr May 16, 2016

Member

@benbalter Random thought: should we suggest Creative Commons here instead?

@parkr

parkr May 16, 2016

Member

@benbalter Random thought: should we suggest Creative Commons here instead?

@parkr parkr changed the title from WIP: Add 'jekyll new-theme' command to Add 'jekyll new-theme' command to help users get up and running creating a theme May 19, 2016

# Specify a layout from your theme!
---
Lorem ipsum dolor sit amet, quo id prima corrumpit pertinacia, id ius dolor dolores, an veri pertinax explicari mea. Agam solum et qui, his id ludus graeco adipiscing. Duis theophrastus nam in, at his vidisse atomorum. Tantas gloriatur scripserit ne eos. Est wisi tempor habemus at, ei graeco dissentiet eos. Ne usu aliquip sanctus conceptam, te vis ignota animal, modus latine contentiones ius te.

This comment has been minimized.

@parkr

parkr May 19, 2016

Member

@benbalter When a user creates a new theme using this command, they get a bundle exec rake preview command for free, which shows them this example website. Should we use this page to explain what to do, or just leave it to the README?

@parkr

parkr May 19, 2016

Member

@benbalter When a user creates a new theme using this command, they get a bundle exec rake preview command for free, which shows them this example website. Should we use this page to explain what to do, or just leave it to the README?

@parkr parkr added the feature label May 19, 2016

@parkr parkr added this to the 3.2 milestone May 19, 2016

@parkr

This comment has been minimized.

Show comment
Hide comment
@parkr

parkr May 20, 2016

Member

@jekyllbot: merge +minor

Member

parkr commented May 20, 2016

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit ee2c41a into master May 20, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jekyllbot jekyllbot deleted the new-theme-command branch May 20, 2016

jekyllbot added a commit that referenced this pull request May 20, 2016

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter May 22, 2016

Contributor

@parkr Nice work. Ran things locally, and everything worked as expected, but had some feedback on the experience and on the defaults:

spec.metadata["plugin_type"] = "theme"

How is this used? I remember there was a discussion about this when the feature was originally proposed, but depending on its use, wondering if we should namespace things in a jekyll namespace. It looks like values must be strings, but maybe jekll_plugin_type?

spec.bindir = "exe"

When would a theme have an executable? (From the default gemspec)

spec.files =git ls-files -z.split("\x0").reject { |f| f.match(%r{^(exe|_layouts|_includes|_sass|example|example/_posts)/}) }

Unless I'm misreading, we want to include layouts/includes/sass, the gemspec and exclude everything else (including exe). (A) I don't think this line accomplishes what we want right now, and (B) I'm wondering if we should default to a whitelist to encourage stronger defaults.

Rakefile... example dir

This feels super heavyweight to me, and given that we have no way to e.g., distribute bug fixes to themes, I think we're setting ourselves up for heartbreak and failure "To fix this issue, copy and paste this Gist into your theme's Rakefile" /headdesk.

I'm not sure the example dir + Rakefile is the best approach.... it feels a bit heavy handed and adds a lot of clutter for theme maintainers (whom we should assume are new to Ruby). Based on the theme docs, they should be able to simple add page.md, index.html, or any other file that they want and just run jekyll serve. The theme should be a valid Jekyll site, so that we use Jekyll's existing tooling, rather than having each theme build their own.

Contributor

benbalter commented May 22, 2016

@parkr Nice work. Ran things locally, and everything worked as expected, but had some feedback on the experience and on the defaults:

spec.metadata["plugin_type"] = "theme"

How is this used? I remember there was a discussion about this when the feature was originally proposed, but depending on its use, wondering if we should namespace things in a jekyll namespace. It looks like values must be strings, but maybe jekll_plugin_type?

spec.bindir = "exe"

When would a theme have an executable? (From the default gemspec)

spec.files =git ls-files -z.split("\x0").reject { |f| f.match(%r{^(exe|_layouts|_includes|_sass|example|example/_posts)/}) }

Unless I'm misreading, we want to include layouts/includes/sass, the gemspec and exclude everything else (including exe). (A) I don't think this line accomplishes what we want right now, and (B) I'm wondering if we should default to a whitelist to encourage stronger defaults.

Rakefile... example dir

This feels super heavyweight to me, and given that we have no way to e.g., distribute bug fixes to themes, I think we're setting ourselves up for heartbreak and failure "To fix this issue, copy and paste this Gist into your theme's Rakefile" /headdesk.

I'm not sure the example dir + Rakefile is the best approach.... it feels a bit heavy handed and adds a lot of clutter for theme maintainers (whom we should assume are new to Ruby). Based on the theme docs, they should be able to simple add page.md, index.html, or any other file that they want and just run jekyll serve. The theme should be a valid Jekyll site, so that we use Jekyll's existing tooling, rather than having each theme build their own.

@benbalter

This comment has been minimized.

Show comment
Hide comment
@benbalter

benbalter May 22, 2016

Contributor

One other thing to note, we may not want to add a code of conduct by default. Having a code of conduct that the author doesn't have any intention to support could send the wrong signal... that those codes of conduct are intended to protect should contribute to this project (thinking it's safe and that they'd be protected), when in fact, that's not true.

I'd definitely be in favor of a --code-of-conduct flag, or something similar, especially for a first pass, but don't know the creating a situation where a might run a command and publish a repo without knowing that my project even has a code of conduct (or what the contributor covenant is), is the best idea.

Contributor

benbalter commented May 22, 2016

One other thing to note, we may not want to add a code of conduct by default. Having a code of conduct that the author doesn't have any intention to support could send the wrong signal... that those codes of conduct are intended to protect should contribute to this project (thinking it's safe and that they'd be protected), when in fact, that's not true.

I'd definitely be in favor of a --code-of-conduct flag, or something similar, especially for a first pass, but don't know the creating a situation where a might run a command and publish a repo without knowing that my project even has a code of conduct (or what the contributor covenant is), is the best idea.

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