Repository navigation
Fix factual errors, stale source links and broken anchors in the 2.x docs - #592
karl-bullock wants to merge 3 commits into
Conversation
…docs Audited the docs/ tree against flarum/[email protected] and corrected what could be verified against the source. Wrong API references: model-visibility.md attributed a scoper sample to the non-existent Flarum\Post\PostPolicy (it is Flarum\Post\Access\ScopePostVisibility); api.md said removeField where the method is removeFields; forms.md credited the external m.attrs.bidi library for what is now core's own common/utils/bidi; contributing.md still said PHP 7 type hinting; testing.md linked a removed PHPUnit annotations page; models.md had two examples using parenthesis-free new, which only parses on PHP 8.4 and not on the 8.3 floor. Stale source links: 22 links pointed into flarum/framework on main, whose HEAD predates 2.0, and 15 at the bundled extension repos on master, the 1.x subtree split. All repointed to 2.x, with real replacements for three files renamed or removed since 1.x. Broken anchors: 19 internal links pointed at anchors that do not exist, including six in i18n.md and two references to a heading that exists nowhere in the docs. Laravel links: core requires illuminate/* ^13.0, so the 12.x (and stray 9.x/10.x/11.x) documentation links now point at 13.x. install.md: MySQL minimum corrected to 5.7.8 to match DatabaseRequirements, the PostgreSQL and SQLite PDO drivers noted alongside pdo_mysql, and the Caddy example moved off a PHP 7.4 socket. Also fixed dead external links where the correct target was unambiguous.
|
Some existing issues I only found after opening this, in case they help with triage:
Unrelated to the diff, but found while checking the same pages: |
The HTML tags section said "not all tags are passed as an argument, only those who have attributes". Attributes are not what decides it. Translator::autoProvidedTags() is a closed list (strong, code, i, s, em, sup, sub) that Flarum fills in for you; any other tag, including the <a> in the very example on that page, renders nothing unless a matching parameter is passed. Attributes are instead the reason to pass a parameter for a tag that is already on the list.
Relatedly, attributes in a locale string are not supported at all: preprocessTranslation() strips them and fires a debug warning, and no bundled locale file carries any. That constraint was undocumented, so it is now stated where someone writing a translation will meet it.
Also documented two things the page did not mention. trans() returns Mithril content rather than a string, so its third argument and flarum/common/utils/extractText are the way to get a plain string for an attribute, a title or a select option. And 'user' is a reserved parameter name: the translator extracts it as a User model and derives username from it, so {user} resolves to nothing, which is a confusing half hour for anyone who picked it as a variable name.
|
Pushed one more commit here, from a closer read of
One thing I checked and deliberately left alone: the example passes the tag parameter as a vnode ( |
The PostLikedBlueprint sample declares none of the five methods' return types, while BlueprintInterface declares one on every method: ?AbstractModel, ?User, mixed, string and string. PHP permits a return type to be narrowed, never widened, and an omitted type counts as wider than a declared one, so copying the example gives "Declaration of PostLikedBlueprint::getSubject() must be compatible with BlueprintInterface::getSubject(): ?AbstractModel" rather than a working blueprint. Verified by compiling both the old and the corrected form. The sample now matches the real class in flarum/likes, which also means constructor property promotion in place of two hand-declared properties, getData() returning null rather than falling off the end, and the AbstractModel import that getSubject()'s return type needs. A note explains why the types cannot be dropped, since omitting a return type reads like a tidiness choice rather than a hard requirement. Everything else checked on this page: the driver example's send() and registerType() signatures already match NotificationDriverInterface exactly, and all four documented mail templates (mail::html.notification, mail::plain.notification, mail::html.information, mail::plain.information) exist as real view files.
|
One more commit here, found by a check worth describing because it is mechanical and repeatable. After #603 turned up a documented class whose method signature was incompatible with the interface it implements, I extracted every documented
The blueprint declares none of its five return types, while I confirmed that by compiling both the old and the corrected form against a stand-in of the real interface. Worth stressing that The sample now matches the real class in It is folded into this PR rather than opened separately because this branch already edits the lines just above that code block (the Two things on that page needed nothing, which is the useful half: the driver example's No other page in the docs has a signature mismatch. |
This branch has not been deployed
1 participant
I audited the 2.x documentation (the
docs/tree) againstflarum/[email protected]ated8a525and fixed what I could verify as wrong. Every claim below was checked against the source rather than assumed.Wrong API references
extend/model-visibility.mdattributed a code sample toFlarum\Post\PostPolicy, which does not exist. The sample is verbatim fromFlarum\Post\Access\ScopePostVisibility, and since the section is about scopers rather than policies, I repointed it there.PostPolicydoes no query scoping at all.extend/api.mdsaid fields are removed "through theremoveFieldmethod", but the extender method (and the code block directly beneath that sentence) isremoveFields.extend/forms.mdsaid Flarum patches Mithril with the externalm.attrs.bidilibrary. In 2.xbidiis part of core atframework/core/js/src/common/utils/bidi.js, applied bypatchMithril.js, and is not a package dependency. The link was also truncated mid-URL with no closing parenthesis, and the repo it pointed at is gone.contributing.mdsaid "We use PHP 7 type hinting". Core requires^8.3.extend/testing.mdlinked a "PHPUnit annotations guide" in the same sentence that explains annotations no longer register tests, and that page is a 404 under PHPUnit 12. It now points at the attributes guide.extend/models.mdhad two relationship examples written asnew Extend\Model(User::class)->hasOne(...). Parenthesis-freenewrequires PHP 8.4, so those examples were a parse error on 8.3, which is the supported floor. I wrapped them to match every other example on the page.Stale source links
22 links pointed into
flarum/frameworkonmain. That branch's HEAD is "Apply fixes from StyleCI" from 2024-06-21, so it serves 1.x-era code. Most of those paths still exist on2.x, which means readers were being shown 1.x source for 2.x documentation instead of getting an obvious 404. A further 15 links pointed at the bundled extension repos onmaster, which is the 1.x subtree split (those repos default to2.xnow).All of them now point at
2.x. Three files had been renamed or moved and needed a real target rather than a branch swap:SimpleFlarumSearchTest.phpwas removed in the 2.0 search refactor. The workaround it was cited for (FULLTEXT indexing not happening inside a transaction) now lives inframework/core/tests/integration/api/discussions/ListWithFulltextSearchTest.php.flarum/likesjs/src/forum/index.jsis nowindex.ts.flarum/tagsTagDiscussionModal.jsis nowTagDiscussionModal.tsx.I left
flarum/installation-packages/tree/mainalone (that repo's default branch really ismain), along with the two deliberate1.xlinks in the upgrade guide.Broken internal anchors
19 internal links pointed at anchors that do not exist. Six were in
i18n.mdalone, including#appendix-a:-standard-key-format, which keeps a colon the slugger strips, and#html-tagswhere the heading is "Adding HTML Tags" (line 7 of that same file already used the correct#adding-html-tags).admin.md#telling-the-api-about-your-extensionwas referenced from two pages although no such heading exists anywhere in the docs, so those now point ati18n.md#namespacing-translations, which is where the extension ID format is actually documented.Laravel documentation links
Core requires
illuminate/* ^13.0, but the docs linkedlaravel.com/docs/12.x41 times, plus stray 9.x, 10.x and 11.x links and fourapi/11.xlinks. All now point at 13.x.Dead external links
Fixed where the correct target was unambiguous:
flarum.org/chatandflarum.org/discord/(both 404) todiscord.gg/flarum, Symfony's 5.2 translation pages to their current equivalents, the Intervention Image v3 upgrade guide, theItemListAPI docs link (it used the old ESDoc URL shape rather than the TypeDoc one the rest of the docs use), thesymfony/consoleArrayInputlink, and thedocsURL in thecomposer.mdexample.install.md accuracy
MySQL 5.7+ / 8.0.30+becameMySQL 5.7.8+, which is whatDatabaseRequirements::MYSQL_MINIMUMenforces. I could not find anything in 2.0 that gates on8.0.30.pdo_mysqlon a page that documents SQLite and PostgreSQL support, so I noted that those engines also needpdo_pgsqlorpdo_sqlite.php7.4-fpm.socksocket on a page that requires PHP 8.3+.Things I found but did not change
Flarum\Install\Installation::prerequisites()hard-requires thepdo_mysqlextension even for a SQLite or PostgreSQL install. That looks like a core issue rather than a docs one, so I documented the additional driver requirement and left this for you to judge.curlandsession. The installer checks neither, and I found nocurl_*orsession_*calls in core or the bundled extensions. Guzzle does prefer curl in practice, so I did not want to remove them on my own reading.fluxbb.org(the domain no longer resolves, inREADME.md), the SMF2 migration script repo and thethegeekdiary.compermissions tutorial (install.md), andflarum.org/composer/plusflarum.org/dashboard/subscriptionsinextensions.md(those premium extension instructions look like they predate a flarum.org restructure).console.mddocuments 10 commands.avatars:backfill-variants,avatars:convert-to-webp,schema:dump,extension:enable,bisect,queue:pause,queue:resume,announcements:refreshandextensions:sync-abandonedare all registered but undocumented./php/master/. I checked and that alias does serve 2.x (2.0-only classes resolve, 1.x-only classes 404), so I left them as they are.Verification
Flarum\*class reference in the docs was checked against the 2.x source: 107 distinct references, and the four that do not resolve are all deliberate (aFlarum\Discussionscounter-example incontributing.md, the removedFlarum\QueryandFlarum\Filternamespaces in the upgrade guide, and aFlarum\TYPE\Eventplaceholder).flarum/{common,forum,admin}/...frontend import paths resolve to real files.models.md#adding-new-models-1, which is correct: it is Docusaurus's suffix for the second "Adding New Models" heading.I only touched
docs/, nothing underi18n/orversioned_docs/.