Details

    • Type: Sub-task
    • Status: Unverified Fix
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 3.1.0-dev
    • Fix Version/s: 3.1.0-a1
    • Component/s: None
    • Labels:
      None

      Description

      For phpbb 3.1 we should drop ?> php closing tags in all php files.

      They are not required (http://www.php.net/manual/en/language.basic-syntax.instruction-separation.php) when they are at the end of file. There are multiple reasons to not have them:

      1. No worries about outputting whitespace before headers by accident.
      2. At least the 'patch' utility adds newline at end of file when applying patches.
      3. Various utilities such as sed add newline at end of file, when php files are processed with them.
      4. Diffs without newlines at end of file have an additional line stating the absence of newline.

      There are several posts on the net advocating against using closing php tags:

      http://phpstarter.net/2009/01/omit-the-php-closing-tag/
      http://choosetheforce.blogspot.com/2008/05/should-you-close-that-php-tag.html

      Zend Framework prohibits (http://framework.zend.com/manual/en/coding-standard.php-file-formatting.html) closing php tags.

      Discussion: http://area51.phpbb.com/phpBB/viewtopic.php?f=3&t=32589

      1. remove-php-end-tags
        1 kB
        Oleg [X]
      2. remove-php-end-tags
        0.8 kB
        Oleg [X]

        Issue Links

          Activity

          Hide
          Oleg Oleg [X] (Inactive) added a comment -

          There's no need to do all of it manually when the job can be done with 37 lines of python or, I'm sure, no more than 3 lines of perl.

          I'm going to attach a python script that handles php files strictly ending in ?> (that is, not ?> and newline). Running it on the current source tree leaves several files that still have ?> in them. These files need to be investigated. Most probably simply have a newline after ?>, which is a trivial fix. If any files mix php and html, removing ?> at the end may look wrong, so such files need to be excepted from the run.

          Show
          Oleg Oleg [X] (Inactive) added a comment - There's no need to do all of it manually when the job can be done with 37 lines of python or, I'm sure, no more than 3 lines of perl. I'm going to attach a python script that handles php files strictly ending in ?> (that is, not ?> and newline). Running it on the current source tree leaves several files that still have ?> in them. These files need to be investigated. Most probably simply have a newline after ?>, which is a trivial fix. If any files mix php and html, removing ?> at the end may look wrong, so such files need to be excepted from the run.
          Hide
          Oleg Oleg [X] (Inactive) added a comment -

          Python 2.6 script to kill ?> when strictly at the end of php files, works on files and trees.

          Show
          Oleg Oleg [X] (Inactive) added a comment - Python 2.6 script to kill ?> when strictly at the end of php files, works on files and trees.
          Hide
          rma-web rma-web added a comment -

          Honestly did not think of using a file to automate the process.

          Show
          rma-web rma-web added a comment - Honestly did not think of using a file to automate the process.
          Hide
          Oleg Oleg [X] (Inactive) added a comment -

          Version 2 of the removing script. -a option will also remove tags which are followed by whitespace. Python 2.5 and possibly earlier compatibility.

          Show
          Oleg Oleg [X] (Inactive) added a comment - Version 2 of the removing script. -a option will also remove tags which are followed by whitespace. Python 2.5 and possibly earlier compatibility.
          Hide
          igorw Igor Wiedler [X] (Inactive) added a comment -

          I also added a trailing newline to add files.

          Show
          igorw Igor Wiedler [X] (Inactive) added a comment - I also added a trailing newline to add files.

            People

            • Assignee:
              igorw Igor Wiedler [X] (Inactive)
              Reporter:
              Oleg Oleg [X] (Inactive)
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development