fix: doc generator builder case #123
Conversation
Case of 'Pdf' and 'Screenshot' causes broken links in generated documentation
| @@ -20,15 +20,15 @@ | |||
| * @var array<string, non-empty-list<class-string>> | |||
| */ | |||
| const BUILDERS = [ | |||
| 'Pdf' => [ | |||
| 'pdf' => [ | |||
There was a problem hiding this comment.
Pdf should be kept since it's used to generate the title.
pdf and screenshot folders should be renamed to fix links. Also, the CI check should use git status --short --renames instead to check the diff. git diff --name-only does not seem to detect file renaming.
There was a problem hiding this comment.
If renaming pdf and screenshot, should the async folder also be renamed to Async for consistency?
There was a problem hiding this comment.
Good catch! We can also fix this folder.
Or, in order to keep folders in lowercase, the BUILDERS constant could keep lowercase keys but using ucfirst or ucwords when writing the title. WDYT ?
There was a problem hiding this comment.
ucfirst or ucwords was my first thought, but looking at the other folders there's a mix of snake case, Pascal case and lowercase, and even the folder hierarchy is different for /docs/. Maybe a new issue should be opened to decide on consistency for cases of file / folder names.
I was hoping to suggest a quick fix for the broken links but I seem to have opened a can of worms!
/docs/pdf/builders_api/HtmlPdfBuilder.md - lower case, snake case
/src/Builder/Pdf/HtmlPdfBuilder.php - Pascal case
/tests/Builder/Pdf/HtmlPdfBuilderTest.php - Pascal case
/test/Fixtures/pdf/ - Pascal case, lower case
There was a problem hiding this comment.
IMO, /docs/pdf/builders_api/HtmlPdfBuilder.md should be the correct way since only HtmlPdfBuilder refers to a classname. So, the ucfrist solution is the best for now.
Concerning /test/Fixtures/pdf/, you're probably right because this folder does not contains any class to load use PSR4. It could be done in a dedicated PR.
Case of 'Pdf' and 'Screenshot' causes broken links in generated documentation