Redundant php open and close tags in garland theme. [#2868584]

Redundant php open and close tags in Garland theme page templates php file.They should either be removed or appropriate documentation should be added to it.

Comment	File	Size	Author
#5	drupal7-extra-php-tags-2868584-5.patch	11.76 KB	cchoudhary
#5	7.x: PHP 7 & MySQL 5.5 2,038 pass PHP 5.3 & MySQL 5.5 2,149 pass PHP 7.4 & MySQL 5.7 2,149 pass PHP 7.4 & PostgreSQL 9.5 2,110 pass PHP 7.4 & SQLite 3.27 2,145 pass PHP 8.1 & MySQL 5.7 2,149 pass
#2	drupal7-extra-php-tags-2868584-2.patch	922 bytes	gaurav.kapoor
#2	7.x: PHP 5.3 & MySQL 5.5 2,034 pass

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

11 April 2017 at 09:23

gaurav.kapoor created an issue. See original summary.

Comment #2

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented 11 April 2017 at 09:24

Status:

Active

» Needs review

File	Size
drupal7-extra-php-tags-2868584-2.patch	922 bytes
7.x: PHP 5.3 & MySQL 5.5 2,034 pass

Comment #3

gaurav.kapoor CreditAttribution: gaurav.kapoor at OpenSense Labs commented 11 April 2017 at 10:00

Assigned:

gaurav.kapoor

» Unassigned

Comment #4

Chi CreditAttribution: Chi commented 19 July 2017 at 10:20

I propose instead of removing it we place file documentation within these tags. Just as it is done in Bartik's templates.

Comment #5

cchoudhary CreditAttribution: cchoudhary at ]init[ AG commented 22 January 2018 at 06:53

File	Size
drupal7-extra-php-tags-2868584-5.patch	11.76 KB
7.x: PHP 7 & MySQL 5.5 2,038 pass PHP 5.3 & MySQL 5.5 2,149 pass PHP 7.4 & MySQL 5.7 2,149 pass PHP 7.4 & PostgreSQL 9.5 2,110 pass PHP 7.4 & SQLite 3.27 2,145 pass PHP 8.1 & MySQL 5.7 2,149 pass

Here is the patch, with file documentation(same as Bartik), instead of removing php tags.

Comment #6

cchoudhary CreditAttribution: cchoudhary at ]init[ AG commented 22 January 2018 at 06:53

Comment #7

poker10 CreditAttribution: poker10 at ActivIT s.r.o. commented 7 January 2023 at 18:40

Status:

Needs review

» Needs work

Thanks for working on this!

Personally I think that the approach which adds the documentation is better (#5). However there seems to be some problems:

+++ b/themes/garland/page.tpl.php
@@ -1,4 +1,89 @@
+ * Regions:
+ * - $page['header']: Items for the header region.
+ * - $page['featured']: Items for the featured region.
+ * - $page['highlighted']: Items for the highlighted content region.
+ * - $page['help']: Dynamic help text, mostly for admin pages.
+ * - $page['content']: The main content of the current page.
+ * - $page['sidebar_first']: Items for the first sidebar.
+ * - $page['triptych_first']: Items for the first triptych.
+ * - $page['triptych_middle']: Items for the middle triptych.
+ * - $page['triptych_last']: Items for the last triptych.
+ * - $page['footer_firstcolumn']: Items for the first footer column.
+ * - $page['footer_secondcolumn']: Items for the second footer column.
+ * - $page['footer_thirdcolumn']: Items for the third footer column.
+ * - $page['footer_fourthcolumn']: Items for the fourth footer column.
+ * - $page['footer']: Items for the footer region.

Garland theme does not define any regions in the .info file, so I suppose that only default regions will be available in the page.tpl.php (not all as in the Bartik theme) - see: https://www.drupal.org/docs/7/theming/default-info-values

Also I think it would be better to check the coding standards in the comments, as it seems to me that some comments are longer than 80 characters (see: https://www.drupal.org/node/1354).

Thanks!