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

Clean up Item dates #22

Open
kevinrenskers opened this issue Feb 17, 2025 · 3 comments
Open

Clean up Item dates #22

kevinrenskers opened this issue Feb 17, 2025 · 3 comments

Comments

@kevinrenskers
Copy link
Member

kevinrenskers commented Feb 17, 2025

Right now an Item has three date properties:

var published: Date { get set }
var created: Date { get set }
var lastModified: Date { get set }

This seems rather silly. RSS/Atom feeds need the lastModified date, so it makes sense to keep this.

But as for published date, and the created date... they seem to mostly serve the same purpose. And the readers are trying to read a data properly from the markdown frontmatter to fill the published property, even though Saga has absolutely no default handling for any other metadata. And every reader now has to deal with this, try to read a date from the frontmatter.

I think the readers should only return a title, the HTML body, and the metadata as a [String: String] dict instead of an entire Item instance, let Saga deal with all those properties and the metadata, let it create the Item instance. A lot less repetition in the readers, and it makes it a lot easier to make changes in Saga without having to then also modify every reader.

I think published should be removed, along with the default frontmatter handling. If you want to have a date or published field in your Item, simply add it to the strongly typed metadata, and the metadata parser will take it from there. No magic handling of a single data property.

As for created, not sure if I should keep it. It doesn't hurt to keep it, but on the other hand since this date is so unreliable (see #21), maybe best to not expose it by default.

Obviously this would be a breaking change.

@kevinrenskers
Copy link
Member Author

Working on this, and I have some thoughts.

  1. I am keeping lastModified for easy access
  2. I renamed published to date
  3. I removed created

I kept date (which defaults to the file creation date) since we have the yearWriter and the publicationDateInFilename functions which need some kind of date property in the Item itself. They can't rely on there being a data or published properly in the metadata, which is just a generic M for them. The reason why I renamed published to date is because published has more meaning. Like, it implies that if this date is set in the future, that the item won't be published yet, while Saga doesn't do this by itself (you can use a filter in the register step to do this yourself though).

Since date is such an important property of the Item itself, I also decided to keep the default behavior where Saga tries to fetch the date from the metadata. But! This logic now lives in Saga itself, instead of every Reader having to do this.

@kevinrenskers
Copy link
Member Author

The readers got greatly simplified with the recent change, see for example loopwerk/SagaParsleyMarkdownReader@9605b14. Love it!

@loopwerk loopwerk locked and limited conversation to collaborators Feb 24, 2025
@loopwerk loopwerk unlocked this conversation Feb 24, 2025
@kevinrenskers
Copy link
Member Author

I kept date (which defaults to the file creation date) since we have the yearWriter and the publicationDateInFilename functions which need some kind of date property in the Item itself. They can't rely on there being a data or published properly in the metadata, which is just a generic M for them.

Thinking about this some more... these two functions could just take a closure that returns a writable Date for a given Item. That way nothing needs to be hardcoded and the Item doesn't have to ship with a date property. No need to try to read the date from the metadata by itself, when nothing else is done automatically like that.

Not 100% sure about it yet, if it's worth yet another breaking change. Need to play around with this.

@kevinrenskers kevinrenskers reopened this Feb 24, 2025
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

No branches or pull requests

1 participant