Skip to content
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

Conversation to Smarty Template engine (reworked) #16

Merged
merged 4 commits into from Jan 3, 2020

Conversation

schnoog
Copy link
Contributor

@schnoog schnoog commented Jan 3, 2020

I've gone the way mentioned
#15 (comment)

Applied .editorconfig within VSCode, hopefully everything is now correctly formatted.

@jrie
Copy link
Owner

jrie commented Jan 3, 2020

There seems to be a issue on my testing station regarding the pathes of the language files in includer.php - as the path used starts with dot. require_once($basedir . './vendor/autoload.php');' .

Also I get an error when trying to copy over and check for the modification time of the language file in initLang of language_tools.php .
I guess a fix could be:
From $filename = "$locales_root/$locale/LC_MESSAGES/$domain.mo";
to $filename = "$locales_root/{$locale}/LC_MESSAGES/$domain.mo";

And $stripping the "/" slashes from the $locale path.

$locales_root points to "localhost" in my setup - one option, I guess, could be, removing $basedir from line 3 of the definition of LANGUAGEDIR here:
define('LANGUAGEDIR', $basedir . '/languages/locale/');

Lastly, nav.tpl checks for the {if isset($SESSION.user.usergroupid)} but not if $SESSION.user is set and throws an error.

Could you verify this?

@schnoog
Copy link
Contributor Author

schnoog commented Jan 3, 2020

  • Added check for $SESSION.user in nav.tpl
  • removed the additional "." from autoload including.
  • Changed language_tools.php
$filename = "$locales_root".$locale."/LC_MESSAGES/$domain.mo";
$filename_new = "$locales_root".$locale."/LC_MESSAGES/{$domain}_{$mtime}.mo";

-Updated README.md added (folder permission hint)

Tested it successfully on

  • Windows 10 (XAMPP PHP 7.4.1)
  • Debian 10 (buster) (PHP 7.3.11)
  • Raspbian GNU/Linux 9 (stretch) (PHP 7.0.33)

@jrie
Copy link
Owner

jrie commented Jan 3, 2020

The language switcher does not work in my test, also I don't see any session cookie created for the language selection.

In addition, the following changes had to be made:

  1. Create directory /smartyfiles
  2. Change owner of following files to www-data:
  • chown -R www-data smartyfiles
  • chown -R www-data smartyfolders
  • chown -R www-data languages/locale

Also, we could use the custom dropdown for the language selection itself?

@Darksider3
Copy link
Contributor

Darksider3 commented Jan 3, 2020

Aside from the permissions, the PR is working fine for me. :-)

Regarding the dropdown-thingy, would do that rather in another PR then cuz' it should be at least documented in the commit history? Am in for that in general, seems nice :)

@schnoog
Copy link
Contributor Author

schnoog commented Jan 3, 2020

The language switcher does not work in my test

Have you installed the en_GB locale on your machine?

Create directory /smartyfiles

This folder isn't required on any of my test machines (and also not available within my repo)

chown -R www-data smartyfolders
chown -R www-data languages/locale

Should be mentioned within the README.md

Also, we could use the custom dropdown for the language selection itself?

I first used a dropdown (). But this messed up all the JS Dropdown code on inventory. (Simply "uncomment" the relevant lines in nav.tpl [ you need to remove the whitespaces right to "{" and left to "}" which is a type of smarty escape] to see the "magic" happen.

@jrie jrie merged commit 3e1800f into jrie:master Jan 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants