search

getzola / zola

A fast static site generator in a single binary with everything built-in. https://www.getzola.org
https://www.getzola.org
MIT License
13.44k stars 941 forks source link

Linter for zola #2345

Open c-git opened 10 months ago

c-git commented 10 months ago

I tried looking but couldn't find any "zola aware" linter. I'm looking for the following.

Edit: I am collecting a list of lints in this comment. So if you have ideas please reply and I'll add them there. Happy for any feedback

Lints

I suspect this does not already exist mostly posting here to see if I missed it. I do intend to make one if it doesn't exist. To that end @Keats if it doesn't and I do make it do you think linking to it would be helpful to others?

Keats commented 10 months ago

I'm not aware of anything like that. It's very opinionated but it could potentially be something we add to Zola itself and we make it run as part of zola check

c-git commented 10 months ago

I'm not opposed to that idea but given how opinionated it is might be better to put it as "optional" and let users opt in by enabling it in the config.toml so that it doesn't change the existing behaviour in a surprising way. I actually like the idea of it being integrated as it would make it easier for users to find and make use of. I was thinking of providing a few "builtin" lints and allowing users to add their own using trustfall. I'm new to trustfall and this would be my first time using it but I'm confident based on what I've learned about it so far it's up to the task. It's what powers cargo-semver-checks that is used to check if new releases have any accidental semver violations.

Keats commented 10 months ago

I'm not opposed to that idea but given how opinionated it is might be better to put it as "optional" and let users opt in by enabling it in the config.toml so that it doesn't change the existing behaviour in a surprising way.

Yes it would be definitely an optional setting the config, since every site will have different requirements or even want lints.

I don't know about adding some new query language though. If it starts being something so complicated that it's hard to trivially answer it by having all the data in a struct or is uncommmon enough that it's needed for one person, maybe then they can reach out to other tools and handle it themselves.

c-git commented 10 months ago

To be honest what I want right now is sufficiently easy that I can do it with just a struct but if the increase in executable size is not substantial I was thinking it would be better to make it user extendable. So I wouldn't have to make changes to the zola executable every time I want a new lint. I was thinking of something like how shortcodes work but instead for lints and you just put the trustfall queries in there. So they get the built in ones similar to how you get atom.xml and rss.xml but you can add more or override them. I was thinking something similar to that would make sense for this as it add the least maintenance burden on the project but gives users maximal flexibility to decide what they want.

And then the community can create their own collection of lints like how themes work.

c-git commented 10 months ago

I've been thinking about this especially how I would use it.

Difference in frequency of use

I would want to run my lints much more often than I would want to run the link checker. For example I run the link checker periodically and it only usually catches links that were added since the last time I ran it. It's not often I have a link that used to work break but I actually did have one I discovered recently when tera documentation moved. Further to that for me personally I would want the lints to run every time I run build. I'm willing to accept the additional cost to the build time. But clearly that would need to be optional as that's not something everyone would want.

More often than build

There are actually lots of times I'll want to run lints but not necessarily the whole build. I was thinking about it and maybe add a top level command to run it like zola lint. But I acknowledge that would make the exposed API of zola more complicated and I'm not sure it's worth it. Just having it in build would be enough imo.

Realtime

To be honest I'd want it to run in "real-time" like how rust-analyzer does but I acknowledge that that is a harder task as then you need to try to maintain state to avoid doing work that's already be done. Wouldn't mind giving it a go one day but it's not a need for now. I've never made an LSP and this seems like a good use case. As it would be nice to get auto complete when editing the front matter and config file.

c-git commented 10 months ago

@Keats please give me feedback at your earliest convenience. I can start a thread on discourse if you prefer. I'm going to start development on a branch in my fork and then we'll have something concrete we can look at. I'll move whatever you don't like out before I make the pull request. Or I can create the pull request and leave it as a draft while I work on it.

Specifically I want feedback on the following:

  1. Should I create a draft PR now or wait until it's ready to create a PR?
  2. How do you feel about adding a zola lint command, do you think the only acceptable way is to build it into zola check (personally I don't run zola check often, don't really want to annoy anyone hitting their site too often)?
  3. How strongly opposed are you to the idea of allowing users to be able to create their own lints / adding trustfall (I think I'll create the prototype with trustfall either way so no big rush on this one). If you are strongly opposed I'll rewrite the lints using only structs for the ones that we decide to move into zola?
  4. Your thoughts on my "date update tool" below (put it below because to be honest I'm not sure it's a good fit for zola because right now I don't think zola ever alters your existing files and maybe that's not worth giving up. But I trust your judgment on this more than mine as I've appreciated your ability to have a clear vision for what you want zola to be and not to be over the years.
  5. I'm curious about your thoughts on having Language Server for zola, this one is a bit of a pipe dream at this point as I think it might be substantially more work and may never happen as a result.

Date Update Tool

I have another idea, this one I know I want, but I don't know how many other ppl will want this. To go along with the ability to lint for missing dates I want an option to update the dates automatically. I want this because I want to use the lint but... I have lots of pages and I was putting the dates manually but it's just robot work. I'd pattern it on cargo fix where it checks if the git working tree is clean before making changes. The only problem is even if multiple people want this they may want it to work differently and I only know what I want but part of that is influenced by the fact that I was doing it manually so I'm open to other ways. Below is what I was doing manually that I want to automate. One more thing command name suggestion zola date_update, I don't like it but planning to use it as the name during development so if someone has a better name I'd appreciate it.

Per file included and not excluded do the following

  1. Get the later of date or updated from the front matter. If both empty leave as None.
  2. Compare date from previous step with last modified date in commit history if different to next step otherwise end.
  3. if updated exists then change that to the last commit date (or should this be today's date as they are about to commit and that's going to make them not match again). Then stop. If updated did not exist, then go to next step.
  4. Set date to last commit date and set updated to today's date if last commit date is not today. STOP.
Keats commented 10 months ago

Not too sure about zola lint vs zola check. You could have zola check --lint-only to achieve the same thing. If you are running it in CI though, it's nice to have it as one command (maybe?).

We can keep the discussion on this issue. Before doing a PR though, you can just link to the repo/branch here. I'm curious how that's going to look with trustfall tbh.

For date update: i don't think it should be in zola. Someone opening a file on zola on eg Windows and changing the line endings but with no actual changes to the content would cause a date change. People should not commit empty changes like that but that does happen in practice and I'd rather let people do it by hand.

c-git commented 10 months ago

I've never tried running zola check in CI but yes adding --lint-only works for me and I prefer it to adding zola lint because it makes it more clear that it's part of check. I will try adding zola check to my CI to see how it works.

Ok I'll link the repo here and only create the PR at the end.

Thanks for the feedback. I'll keep the date changes separate. But unless someone wants skipping line ending only commits as not a date change I'll not bother with that when I do the lint.

c-git commented 10 months ago

I haven't started on this yet but it's still pretty high on my todo list cuz I don't like doing it manually. I had a question that I wanted to ask in the meanwhile. I noticed that zola check (according to the help message) does more than just check links (that's all I used it for). It also tries to build the site without rendering it. I'm thinking if we add lint then it will do 3 things:

Given that maybe instead of --lint-only it might be better to add --skip-links-check as this would remove the slow part of the command but leave the rest enabled. This would, even without the lints, be useful (at least for me) in CI. Right now I use zola build to achieve the same thing but it renders the site while that isn't needed as I only need it to check for errors. This is kinda wasted effort as the site is about to be rendered again for publishing.

What do you think about adding --skip-links-check instead of --lint-only?

Keats commented 10 months ago

zola check just loads the site, it doesn't output anything. We do need that info for the link checking (as well as lints I guess?). I'd rather have --skip-link-checking and --skip-linting both

c-git commented 10 months ago

Yep that makes sense. Will go with that.

c-git commented 10 months ago

I finished, well at least a working version, of the tool to set the dates. It also has a check mode that I can use in a pre-push hook to ensure my dates are set the way I want. That turned out to be much more complicated than I expected. With just the three dates (namely the last_commit_date, and the other two from the font matter date and updated), there turned out to be at least 42 test cases of interest. I don't think I will try to include my version of the rules in the linter cuz

  1. I had to make lots of choices and thus they are maybe just specific to me. With 42 cases to consider it's easy to imagine someone else will disagree on at least 1 case.
  2. It's very complicated to be a lint.

So I will just add a lint for checking there is a date not what that date is. I'm going to create a separate comment after this one where I can list the lints I plan to add. So I can add more as time goes on so I don't forget them as I haven't done it yet. As well as allow ppl to give me feedback on them so I can get something more generally applicable than to me only.

c-git commented 10 months ago

List of planned lints

Features to consider (final version may have some removed)

c-git commented 6 months ago

I've not had the opportunity to work on this yet, I still want it and will still try to make time to do it but if someone wants to do it before me that's completely fine.