tapestry-cloud / tapestry

PHP static site generator using the plates template system
https://www.tapestry.cloud/
MIT License
32 stars 1 forks source link

Permalink Clash detector clashes on static files #255

Closed carbontwelve closed 7 years ago

carbontwelve commented 7 years ago

When the clash detector comes across static files such as css/js/img it needs to know that their permalinks are not supposed to be "pretty" (e.g /css/default.css rather than /css/default/index.css).

This is likely a bug in the methods usage of the files getCompiledPermalink method which accepts a boolean. The method should know if the file is static and should therefore not have a "pretty" permalink.

Some linting tools have highlighted that the getCompiledPermalink method appears to break the single responsibility principle and a quick search of its usage within the project show that some times the bool is passed by value and sometimes its defaulted to false. This is likely a hotbed for bugs in the future and needs to be refactored.

An idea would be for getCompiledPermalink to not accept a $pretty argument and instead have this loaded into the File from the projects configuration (the global pretty permalink setting) and over-ridden on a per File basis when it knows it's to be copied (e.g as a static resource via its private toCopy property, or from the files own front matter disabling pretty urls for that file.)

As per usual all of the above requires tests.


Delving into this I notice that by default upon construction of each File its pretty_permalink data value is set to true.

Refactoring Considerations:

carbontwelve commented 7 years ago

A quick fix for this bug would be to pass $file->isToCopy() to the getCompiledPermalink method within the class detector. But this doesn't actually take into consideration the files true "pretty permalink" setting.

carbontwelve commented 7 years ago

This is fixed by #258