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 ability to remove items from a section #75

Merged
merged 5 commits into from
May 12, 2020

Conversation

peteschaffner
Copy link
Contributor

This change adds a PublishingStep that makes it possible to conditionally remove items.

Background

Coming from other static site generators, I have grown accustomed to writing posts labeled as drafts, which will be excluded from a build until marked as ready. This PR is meant to provide the capability to create such a workflow.

I use this feature for my site by adding an optional draft: Bool? item metadata property, and then conditionally run a step that removes such items from the build:

.if(CommandLine.arguments.contains("--removeDrafts"), .removeAllItems(matching: \.metadata.draft == true))

I then finish things off by passing a --removeDrafts flag when deploying my site.

@john-mueller
Copy link
Contributor

👍 I think this is a great addition. Maybe you could also add your method for removing draft files to the /documentation folder.

Copy link
Owner

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

Very nice, thanks a lot for this @peteschaffner! Just made a quick little tweak to use the removeAll(where:) API instead of filter 👍

@peteschaffner
Copy link
Contributor Author

Very nice, thanks a lot for this @peteschaffner! Just made a quick little tweak to use the removeAll(where:) API instead of filter 👍

@JohnSundell: Ah, good call!

👍 I think this is a great addition. Maybe you could also add your method for removing draft files to the /documentation folder.

@john-mueller: Sounds like a good idea, but I'll let @JohnSundell decide if he would like to have this in the docs. In the meantime, you can checkout how I'm using this step here.

@alexito4
Copy link
Contributor

alexito4 commented Apr 6, 2020

I've used this on my website :) another data point that confirms this works 👍

@pjechris
Copy link

That would be useful indeed :)

I came into a bug however: indexes are not rebuilt crashing my app. I made a fix of my own (https://github.com/pjechris/Publish/blob/master/Sources/Publish/API/Section.swift).

@JohnSundell
Copy link
Owner

Ah that's a very good point @pjechris! Could you perhaps include that patch (https://github.com/pjechris/Publish/blob/master/Sources/Publish/API/Section.swift#L112) in this change, @peteschaffner, and add an additional test covering that the indexes have been rebuilt after an item was removed?

@peteschaffner
Copy link
Contributor Author

Ah good catch @pjechris! I included the patch and added an assertion to the existing test a la testSortingItems. @JohnSundell let me know if you you would prefer a separate test.

@peteschaffner
Copy link
Contributor Author

@JohnSundell I'm not sure what that PublishTests.CLITests testProjectGeneration failure is about, as all tests are passing for me. I wonder if it is a Swift version issue (5.2.2 on my machine and 5.1.3 on your Bitrise stack)?

Copy link
Owner

@JohnSundell JohnSundell left a comment

Choose a reason for hiding this comment

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

Fantastic, thanks again @peteschaffner 🎉

@JohnSundell
Copy link
Owner

And yeah, bumping the Swift version on Bitrise fixed the build issue. Will require Swift 5.2 from hereon out.

@JohnSundell JohnSundell merged commit ddf73c2 into JohnSundell:master May 12, 2020
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.

5 participants