Use language.php instead of dummy (only english) language #252
No reviewers
Labels
No labels
Component: User interface
Component: Wymeditor
Help wanted
Level: Difficult
Level: Easy
Level: Moderate
Pagetype: Festival
Pagetype: Mailinglist
Pagetype: Peer reviewed article
Pagetype: Text
Privacy GDPR AVG
status: has conflicts
Status: Needs changes
Status: Needs discussion
Status: Needs review
Status: Ready to merge
Status: Waiting for response
Type: Bug
Type: Enhancement
Type: Question
Usecase: De Stadsbron
Usecase: Koppelting
Usecase: MeetjeStad
Value: Coders
Value: Security
Value: Users
Value: Visitors
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference
harmen/hypha!252
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "Issue/94-Installing-from-git-shows-only-the-English-language"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Fixes #94
Place languages in separate json data file
I left a few comments inline that can further simplify the code more.
@ -87,3 +87,3 @@if ($file != "." && $file != "..") {if (isAllowedFile($dir.'/'.$file)) $index[ltrim($dir.'/'.$file, "./")] = filemtime($dir.'/'.$file);if (isAllowedFile($dir.'/'.$file)) $index[preg_replace("/^\.\//i","",$dir.'/'.$file)] = filemtime($dir.'/'.$file);elseif (is_dir($dir.'/'.$file)) $index = array_merge($index, index($dir.'/'.$file));I think this change is unrelated to the rest of the commit (IIRC it fixes installation of the .htaccess file?). Would be better to have this in a separate commit (separate PR is not needed).
Why do we still need this replacement? Isn't the content of
languages.jsonvalid json as-is?Ah, it seems these
\'is present in thelanguages.jsonfile you added. These were probably needed when the json was contained in a PHP''string, but this is no longer relevant. I would rather remove these backslashes from thelanguages.jsonfile and remove the replacement above.Shouldn't this have a case for
languages.jsonrather thanlanguage.phpnow? And that could just return thefile_get_contents()for the existinglanguages.json?Do we need this check and else? AFAICS we always have a
languages.jsonwheneverlanguages.phpalso exists, so we could just remove the if?@ -87,3 +87,3 @@if ($file != "." && $file != "..") {if (isAllowedFile($dir.'/'.$file)) $index[ltrim($dir.'/'.$file, "./")] = filemtime($dir.'/'.$file);if (isAllowedFile($dir.'/'.$file)) $index[preg_replace("/^\.\//i","",$dir.'/'.$file)] = filemtime($dir.'/'.$file);elseif (is_dir($dir.'/'.$file)) $index = array_merge($index, index($dir.'/'.$file));This commit could probably be set to close #92.
@ -87,3 +87,3 @@if ($file != "." && $file != "..") {if (isAllowedFile($dir.'/'.$file)) $index[ltrim($dir.'/'.$file, "./")] = filemtime($dir.'/'.$file);if (isAllowedFile($dir.'/'.$file)) $index[preg_replace("/^\.\//i","",$dir.'/'.$file)] = filemtime($dir.'/'.$file);elseif (is_dir($dir.'/'.$file)) $index = array_merge($index, index($dir.'/'.$file));This comment is still unresolved.
This is resolved.
This is resolved.
This is partly resolved, but I'll add a new comment for what's left.
There is still one unresolved comment from before, and I added one new comment. Otherwise this looks good.
The commit structure does need some cleanup, since there are now multiple duplicate commits and a merge commit, but that can just be a matter of using github's squash and merge (or something like that) feature when merging.
This now returns the decoded json (e.g. a PHP array), whereas the version that is generated when packing up hypha has this section replaced with actual file data, so that changes the return value. IOW, when you pack up hypha, the code no longer works.
To fix that, this should just return the result of
file_get_contents(), and thejson_decode()should be moved to the caller ofdata()elsewhere. Might be good to also test this packing up (though I'm not 100% sure it is working in master).fixed
@ -87,3 +87,3 @@if ($file != "." && $file != "..") {if (isAllowedFile($dir.'/'.$file)) $index[ltrim($dir.'/'.$file, "./")] = filemtime($dir.'/'.$file);if (isAllowedFile($dir.'/'.$file)) $index[preg_replace("/^\.\//i","",$dir.'/'.$file)] = filemtime($dir.'/'.$file);elseif (is_dir($dir.'/'.$file)) $index = array_merge($index, index($dir.'/'.$file));done
Looks good to merge now. I think this PR will conflict with #280. Probably easy enough to fix, but I'll leave it to @laurensmartina to resolve the conflict and see whether to merge this one first, or first #280 and then fixing this one (might be slightly easier).