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

Updates on PMA\Template class #1671

Merged
merged 4 commits into from
May 21, 2015
Merged

Conversation

gzzhanghao
Copy link
Contributor

Updates on Template class.

  • Add trim method to Template class. Now the usage is changed to:
    Template::get($template_script_name)->render($data, [$trim = true]);
    The $trim parameter in the render function indicate whether it should eliminate white spaces between HTML tags and its values. This feature is set to ON by default; You can disable it by simply passing false.
  • trim is also provided as a static method.
  • Update tests for the Template class.
  • Add templates for db_designer.lib.php.
  • Change extensions of templates to ".pthml".

@Tithugues
Copy link
Contributor

Hi,
Did you create this for a specific need?

include $template;
} else {
throw new \LogicException(
'The template "' . $template . '" not found.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that we should use translation here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is intended more for the developers if they do happen to type a wrong template name, so we may choose to skip translating it. Its thrown again in catch, so if a user does see a Fatal error, can just copy it and report to us, as user not expected to really do something about it.

@atul516
Copy link
Contributor

atul516 commented May 11, 2015

@atul516
Copy link
Contributor

atul516 commented May 15, 2015

@gzzhanghao Please update it such as to have only changes on top of the current master. (so it will just include addition of trim function and all template class unit tests)

madhuracj added a commit to madhuracj/phpmyadmin that referenced this pull request May 18, 2015
Signed-off-by: Madhura Jayaratne <madhura.cj@gmail.com>
@gzzhanghao gzzhanghao force-pushed the template branch 2 times, most recently from 83856cc to 80bd966 Compare May 19, 2015 15:38
- Introduce `trim` method for templates
- Complete template of the designer module
- Update tests for template system
- Use phtml as template files extension

Signed-off-by: Jason <jason.daurus@gmail.com>
@atul516
Copy link
Contributor

atul516 commented May 19, 2015

@gzzhanghao It would have been better had you done separate things in a new PR, but for now, please update the title/description as well for this PR, maybe as mentioned in your commit message.

if (strstr($tab_column[$t_n]["TYPE"][$j], 'char')
|| strstr($tab_column[$t_n]["TYPE"][$j], 'text')
) {
$type = '_char';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must be .= here and at other similar places.

@atul516
Copy link
Contributor

atul516 commented May 20, 2015

Overall looks very good to me, make the changes as mentioned in above comments, and its ready to merge.

@gzzhanghao gzzhanghao changed the title Introducing PMA\Template class Updates on PMA\Template class May 20, 2015
Signed-off-by: Jason <jason.daurus@gmail.com>
<?php if (isset($tables_pk_or_unique_keys[$t_n . "." . $tab_column[$t_n]["COLUMN_NAME"][$j]])) : ?>
<img src="<?php echo $_SESSION['PMA_Theme']->getImgPath('pmd/FieldKey_small.png'); ?>" alt="*" />
<?php else :
$type = 'pma/Field_small';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo here, pma -> pmd..

Signed-off-by: Jason <jason.daurus@gmail.com>
@gzzhanghao
Copy link
Contributor Author

@zixtor The typo problem was fixed in the newest commit. Please merge :-)

@@ -18,7 +18,7 @@ xgettext \
--from-code=utf-8 \
--keyword=__ --keyword=_pgettext:1c,2 --keyword=_ngettext:1,2 \
--copyright-holder="phpMyAdmin devel team" \
`find . -name '*.php' -not -path './test/*' -not -path './po/*' -not -path './release/*' | sort`
`find . -name '*.php' -o -name '*.phtml' -not -path './test/*' -not -path './po/*' -not -path './release/*' | sort`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gzzhanghao One more thing, this find now seems to be resulting in ./test/* files as well, using regex seems ok to me:-
find -regex '..(php|phtml)' -not -path './test/' -not -path './po/' -not -path './release/' | sort

Signed-off-by: Jason <jason.daurus@gmail.com>
@gzzhanghao
Copy link
Contributor Author

@zixtor Thank you for providing a idea on solving this problem. Now the problem should have been fixed :-)

@atul516
Copy link
Contributor

atul516 commented May 21, 2015

Thanks!

atul516 added a commit that referenced this pull request May 21, 2015
Updates on PMA\Template class
@atul516 atul516 merged commit 319285f into phpmyadmin:master May 21, 2015
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.

3 participants