Fix locals config option
#13
Conversation
WalkthroughThe pull request introduces updates to the documentation and configuration files related to localization settings in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FilamentTypesPlugin
participant Config
User->>FilamentTypesPlugin: Call locals()
FilamentTypesPlugin->>Config: Retrieve filament-types.locals
Config-->>FilamentTypesPlugin: Return locals value
FilamentTypesPlugin-->>User: Return locals or defaults
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (3)
src/FilamentTypesPlugin.php (1)
Line range hint
12-12: Consider updating default static locals to match config.The static
$localsproperty defaults to['ar', 'en']while the config defaults toNULL. Consider aligning these defaults for consistency.- protected static array $locals = ['ar', 'en']; + protected static array $locals = [];README.md (2)
91-93: Fix typos and grammar in documentation.There are a few minor issues in the text:
- "applys" should be "applies"
- Remove the forward slash after "file"
-- Publish the config file/ -- Modify the `locals` array to include the two character language code that applys to the language you wish to offer. EG: +- Publish the config file +- Modify the `locals` array to include the two-character language code that applies to the language you wish to offer. For example:
94-96: Add language specifier to code block.The code block should specify the language for proper syntax highlighting.
-``` +'```php 'locals' = ['en'],<details> <summary>🧰 Tools</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> 94-94: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 0a283a68b731220bbde61a18b4e7b9704e116842 and b8bc7a69bbe3969933a66677549a652c76712b22. </details> <details> <summary>📒 Files selected for processing (3)</summary> * `README.md` (5 hunks) * `config/filament-types.php` (1 hunks) * `src/FilamentTypesPlugin.php` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🪛 Markdownlint (0.35.0)</summary> <details> <summary>README.md</summary> 94-94: null Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> <details> <summary>🪛 LanguageTool</summary> <details> <summary>README.md</summary> [uncategorized] ~126-~126: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’). Context: ...scription() ->searchable(), ``` ## Auto Caching on your `.env` add this ```.env CACHE... (AUTO_HYPHEN) --- [grammar] ~195-~195: Consider using either the past participle “appeared” or the present participle “appearing” here. Context: ... ]; } } ``` it will be not appear on the navigation menu by default but y... (BEEN_PART_AGREEMENT) --- [uncategorized] ~195-~195: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short). Context: ...appear on the navigation menu by default but you can change that by just use this me... (COMMA_COMPOUND_SENTENCE) </details> </details> </details> <details> <summary>🔇 Additional comments (1)</summary> <details> <summary>config/filament-types.php (1)</summary> `39-45`: **LGTM! Clear documentation and sensible default.** The updated configuration with NULL default and clear documentation about the expected format (`['en','ar']`) aligns well with the PR objective of fixing the locals config option. </details> </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| } | ||
|
|
||
| public function getLocals(): array | ||
| { | ||
| return self::$locals; | ||
| return $this->locals(); |
There was a problem hiding this comment.
💡 Codebase verification
Type compatibility issue confirmed - locals() can return null
The locals() method can indeed return null when both config('filament-types.locals') is null and $this->locals is null, which is incompatible with the getLocals(): array return type declaration. This needs to be fixed by either:
- Adding null check in
getLocals() - Ensuring
$this->localsis always initialized as an array - Changing return type to
array|null
🔗 Analysis chain
Verify return type compatibility.
The getLocals() method declares : array return type but calls locals() which could return null. Ensure type compatibility is maintained.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential type mismatches in method calls
ast-grep --pattern 'getLocals()' -A 5
Length of output: 39
Script:
#!/bin/bash
# Let's check the class implementation and method signatures
ast-grep --pattern 'class FilamentTypesPlugin {
$$$
}'
# Also check for the locals() method implementation
ast-grep --pattern 'function locals() {
$$$
}'
Length of output: 109
Script:
#!/bin/bash
# Let's try with ripgrep to find the class and method implementations
rg -l "class FilamentTypesPlugin" --type php
# Find the locals method
rg "function locals\(" --type php -A 5
Length of output: 474
|
hi @pschilly can you check the comments of Code Rabbit? and run tests locally because you have a problem with testing |
|
@3x1io seen - pushed fix. |
|
Not sure how to have the bot redo the tests - but I did push a fix for the bug that you mentioned @3x1io |
|
@3x1io: I see that @pschilly has pushed a fix for the issue. Great job! I will review the updated changes. If you found this review helpful, would you consider giving us a shout-out on X? ✅ Actions performedReview triggered.
|
In the current build the "locals" config item does nothing.
This PR fixes that functionality so that one can dynamically assign the preferred locales.
Changes the default config item to:
NULLso as to maintain the existing options of "Arabic, English" but one can change it simply to an array of['en']or others to change those options out.Summary by CodeRabbit
Documentation
Configuration Changes
NULL, clarifying language options.Functionality Updates
localsmethod to retrieve localization settings dynamically rather than through a parameter.