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

Compose command for collections #103

Merged
merged 4 commits into from Dec 3, 2019
Merged

Compose command for collections #103

merged 4 commits into from Dec 3, 2019

Conversation

alzeih
Copy link
Contributor

@alzeih alzeih commented Sep 4, 2019

This allows creating files in collections, including posts and drafts.

jekyll compose "My New Post"
jekyll compose "My New Post" --post

jekyll compose "My new draft" --draft

jekyll compose "My New Thing" --collection "things"

Default front matter can be specified for each collection.

jekyll_compose:
  default_front_matter:
    drafts:
      layout: post
      category:
      tags:
    posts:
      published: false
      sitemap: false
    things:
      layout: thing

Closes #84

@alzeih
Copy link
Contributor Author

alzeih commented Sep 4, 2019

This PR is related to #84

@DirtyF DirtyF requested a review from a team September 4, 2019 12:46
@ashmaroli
Copy link
Member

Not so sure about what this would mean for the invocation jekyll post and jekyll draft.

Also, drafts and posts are part of the same "collection" (which is posts). There are no tests included in this PR, so can't tell with certainty if things work as expected..

@alzeih
Copy link
Contributor Author

alzeih commented Sep 5, 2019

So, eventually the jekyll compose command could replace jekyll post and jekyll draft as jekyll compose creates both posts and drafts.

I didn't want to remove jekyll post or jekyll draft with this PR because I thought those commands might first become deprecated, and then removed in a later version, if the unified jekyll compose was accepted.

jekyll publish and jekyll unpublish on the other hand are currently very posts collection specific because they move files between _posts and _drafts.

If collections are now to be supported, then publish and unpublish could also be made aware of collections, and instead use the yaml front matter to publish and unpublish collection items. Alternatively, publish and unpublish could just return an error and not act on collection items.

Since @ashmaroli says drafts are not a true collection, the use of jekyll compose --collection drafts could become confusing as a default syntax. I could add flags jekyll compose --postand jekyll compose --draft that alias jekyll compose --collection posts and jekyll compose --collection drafts maybe?

@alzeih alzeih force-pushed the patch-9 branch 2 times, most recently from 02a3666 to 62b0ddd Compare September 5, 2019 03:59
@alzeih
Copy link
Contributor Author

alzeih commented Sep 5, 2019

@ashmaroli I have added tests for jekyll compose.

The tests are from spec/post_spec.rb and spec/draft_spec.rb, and duplicated to test the different arguments and options. There's some small changes to the tests for the different args and options for the commands, and an extra test related to this.

The only major change from the other tests was the structure of default yaml front matter, because I expect it in a different structure, so

jekyll_compose:
  draft_default_front_matter:
    description:
  post_default_front_matter:
    description:

becomes

jekyll_compose:
  default_front_matter:
    drafts:
      description:
    posts:
      description:
    things:
      description:

Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

Some changes need to be made..:

lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
lib/jekyll/commands/compose.rb Show resolved Hide resolved
lib/jekyll/commands/compose.rb Show resolved Hide resolved
lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
spec/compose_spec.rb Outdated Show resolved Hide resolved
@ashmaroli
Copy link
Member

The only major change from the other tests was the structure of default yaml front matter

Please use the existing YAML front matter structure as currently documented in the master branch. Changing the structure (affecting existing commands and usage consistently) should be in a separate PR.

@alzeih
Copy link
Contributor Author

alzeih commented Sep 6, 2019

The only major change from the other tests was the structure of default yaml front matter

Please use the existing YAML front matter structure as currently documented in the master branch. Changing the structure (affecting existing commands and usage consistently) should be in a separate PR.

I have created PR #104 for this.

I believe I have addressed all the changes requested, and will re-request review. I rebased and squashed this PR to incorporate all the changes. Thank you for the review!

lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
lib/jekyll/commands/compose.rb Outdated Show resolved Hide resolved
@ashmaroli
Copy link
Member

@alzeih I took the liberty of pushing further changes myself instead of asking you to do it.
You can update your local version of this branch by running the following:

git pull origin patch-9

alzeih and others added 4 commits September 7, 2019 23:00
@alzeih
Copy link
Contributor Author

alzeih commented Sep 7, 2019

Rebased for conflicts with master branch .rubocop_todo.yml

Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Fine by me. I wish I could simply type jekyll compose _things/my-new-thing.md à la Hugo though.

@alzeih
Copy link
Contributor Author

alzeih commented Sep 7, 2019

@DirtyF I chose name over filename due to how jekyll post and jekyll draft worked. I thought it had the advantage of the auto slugging the title and populating the yaml front matter.

What features do you see jekyll compose _things/my-new-thing.md do above say $EDITOR _things/my-new-thing.md ? (I haven't used Hugo or know what you like about it)

@DirtyF
Copy link
Member

DirtyF commented Sep 7, 2019

@alzeih Not a problem. It's just a personal preference, mainly because I find it way easier to type and memorize.

Hugo has archetypes (it's a whole different approach to provide similar feature)

@alzeih
Copy link
Contributor Author

alzeih commented Sep 7, 2019

@DirtyF I can see the advantage of having automatically selected and prepopulated templates for collection items, including content! I'll think about how it might be done, though probably not for this PR.

@alzeih
Copy link
Contributor Author

alzeih commented Sep 19, 2019

This PR is marked as approved, but has not been merged. Are there any outstanding issues I can address?

@ashmaroli
Copy link
Member

@alzeih This PR will be merged after 1 more approval from @jekyll/plugin-core

@DirtyF
Copy link
Member

DirtyF commented Dec 3, 2019

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit 899b37c into jekyll:master Dec 3, 2019
jekyllbot added a commit that referenced this pull request Dec 3, 2019
@jekyll jekyll locked and limited conversation to collaborators Dec 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: generic compose command
4 participants