WPTT / theme-sniffer

Theme Sniffer plugin using sniffs.
MIT License
270 stars 3 forks source link

Required files check #128

Open ernilambar opened 5 years ago

ernilambar commented 5 years ago

Related to https://github.com/WPTRT/theme-sniffer/issues/115

timelsass commented 5 years ago

If there's a standard documented, then it should be followed is my thought.

For png/jpg - this has been acceptable in the past, but the requirement for this was being based off of https://developer.wordpress.org/themes/release/required-theme-files/ verbatim.

Given that there's actually a screenshot_url in the .org themes api - why even limit screenshots to a particular format, and allow an author to use a format of their liking. Theme sniffer could allow multiple, and warn for preferred format. There should be a standard to the format though as everything else does. I'm not sure how we would go about updating that page, but if the standard were to change from screenshot.png to allow for either or screenshot.jpg then I think we could change it. I also think it could stand to be more permissive - perhaps webp as the recommended format (requiring a fallback), which would not only help keep distribution weight a tiny bit smaller, but help reduce .org resource usage over time. The screenshot is pretty much just for being displayed on .org. We could always have a fix for this sniff which would convert invalid types into the required type so the proper format is only a click away.

For the readme, this was used for reference: https://make.wordpress.org/themes/handbook/review/required/#readme-txt-file

I have an issue with calling a README.md file the same thing as a readme.txt, it's misleading. Extensions are used to determine content by lots of applications that people working with this tool are likely to be using - such as ides, code editors etc. You add unnecessary overhead by trying to parse a .md when it's not even an actual .md, and conflicts. Sure now I have the option to say this a readme.txt file, but it becomes a nuisance opening other projects causing some lag for some of the processes when there was no reason to because the editor recognized it as a particular file which has expected behaviors. Github for instance will attempt to read the .md file and display a render of the contents. This doesn't help out anything when the standards aren't even a standard format that anyone recognizes or is really able to be parsed. Which is the first big problem I believe this format has.

I know the "parser" can attempt to figure out some markdown, but there's so much overhead in the whole process. There's checks for single one-off use cases, which help cover converting some usecases, but not others that would be logically used.

Take the following example of using setext style headers:

Testing Plugin
===========

There's code in the .org script to check for this specific scenario, so it gets formatted the same as the standard:

=== Testing Plugin ===

An author who is using setext style they used it for a reason, so it's safe to assume they are going to write other content in their readme using it, and logically that would be for sections:

FAQ
------

This doesn't validate as the section and you'll have a notice for missing FAQ section. So the only acceptable place for setext style headers is the plugin name, which is weird.

There's a lot of inconsistencies overall, and I've looked a lot of readmes being approved, and can see the issue beginning to crop up. If we want to parse and validate resources - we have to enforce a standard format. People are doing all sorts of weird and/or creative things - some aren't even markdown, some are to make things prettier to them, and most don't even have all of the same information. Regardless we'll end up with an impossible attempt and dirty regex match that's always going to be error prone that no one will work with because it sucks.

Why aren't there any editor plugins that format your WordPress readmes? Why aren't there any build tools to lint WordPress readmes?

There's some that "lint", and by that I mean it just checks for some of the required fields via regex, but it won't tell you if you've formatted outside of a standard spec, which is a major problem if we want to glean information from it and provide warnings, errors, and fixes in a linting fashion.

If you really sit down and think about it - if the readme file was a structured format, this problem wouldn't exist. It's because we allow variations of random things that are inconsistent, such as invalid file types

Taking the example code above show clear evidence:

=== Testing Plugin === Requires at least field: 5.0

== Changelog == =1.0=

this is how github displays the actual markdown if used in a file with the extension .md, or in allowed areas, such as comments:

Testing Plugin

Requires at least field: 5.0

Changelog

1.0

The lax readme format does make things like building an actual parser or linter a bit ridiculous, especially given that .org's "parser" is more so a "formatter and renderer all in one" not a parsersince we can't really gather any token or AST. This is what the other sniffs are doing and are able to do, so a sniff on a readme should be the same. Success for other package managers has been found by using universally recognized way of describing projects in a repo, such as inclusion of that information in a .json or .yaml. WordPress' known type has been readme.txt, so if we're considering making the standard .md and allowing variations of naming conventions, then I would urge moving to one of those formats instead :P

I do have some changes that I think would make sense as I started looking into the requirements, initial validation checks for resources' licenses, and the readme.

If you have any interest in this - or anyone else for that matter, this is my initial WIP.

Some changes that aren't documented in that initial draft include:

carolinan commented 5 years ago

Looks like the directory requires the sceeenshot to be a png:

https://meta.svn.wordpress.org/sites/trunk/wordpress.org/public_html/wp-content/plugins/theme-directory/class-themes-api.php

carolinan commented 5 years ago

I am for requiring readme.txt to start with, regardless of the format. We can keep working on the format.

joyously commented 5 years ago

Looks like the directory requires the sceeenshot to be a png

It's not required. My theme uses jpg, and I know a bunch of themes do also, especially old ones.

dingo-d commented 5 years ago

Yeah, just looked at my theme and it's jpg 🙂

joyously commented 5 years ago

We should require readme.txt and ignore readme.md.

dingo-d commented 5 years ago

I agree, if we want to encourage people to write uniform readmes, we can notify if the readme.txt is missing or not. I wouldn't forbid the readme.md but it shouldn't be checked imo.