-
Notifications
You must be signed in to change notification settings - Fork 821
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Docs for resource loading upgrade path #7076
Docs for resource loading upgrade path #7076
Conversation
|
||
Usage with custom logic: | ||
|
||
```php |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really wish we could use diff
syntax here, but the underlying syntax highlighter used for docs.ss.org doesn't support it... (also: silverstripe/doc.silverstripe.org#157)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bah - does parsedown not support diff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsedown doesn't do syntax highlighting? We're using https://github.com/google/code-prettify as a JS lib, and that doesn't do diff: googlearchive/code-prettify#62. Ideas?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doh, of course. I feel like we have had this chat before, would need a separate JS lib just for diff highlighting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
highlight.js supports a large range of syntaxes, including diff - perhaps we could switch?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use SilverStripe\Core\Manifest\ModuleLoader; | ||
use SilverStripe\View\ThemeResourceLoader; | ||
$moduleFilePath = ModuleLoader::getModule('silverstripe/framework')->getRelativeResourcePath('MyFile.php'); | ||
$baseFilePath = Director::baseFolder() . '/composer.json'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not quite sure if this is the right way to do this now? It sure beats BASE_PATH
, but it's still static.
docs/en/04_Changelogs/4.0.0.md
Outdated
$themeFolderPath = ThemeResourceLoader::inst()->getPath('simple'); | ||
``` | ||
|
||
To ensure consistency, we've also removed support for path constants: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We haven't removed these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced "removed" with "deprecated"
ab17d32
to
f088ee7
Compare
docs/en/04_Changelogs/4.0.0.md
Outdated
* `FRAMEWORK_ADMIN_DIR` and `FRAMEWORK_ADMIN_PATH` | ||
* `FRAMEWORK_ADMIN_THIRDPARTY_DIR` and `FRAMEWORK_ADMIN_THIRDPARTY_PATH` | ||
* `THIRDPARTY_DIR` and `THIRDPARTY_PATH` | ||
* `ASSETS_DIR` and `ASSETS_PATH` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we have a "ASSETS_PATH
" env lookup in config f or assets store configuration, but we could still deprecate it I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, it's too early to deprecate that - there's no appropriate getter. Which we'll need to solve, but it's out of scope of this docs-focused PR. I've removed this line
f088ee7
to
2495741
Compare
See #7065