Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-11632

Multiple files with emtpy lines before <?php and after ?> in CVS.

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 1.7.2, 1.8.2, 1.9
    • Fix Version/s: 1.7.3, 1.8.3, 1.9
    • Component/s: Other
    • Labels:
      None
    • Affected Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE
    • Fixed Branches:
      MOODLE_17_STABLE, MOODLE_18_STABLE, MOODLE_19_STABLE

      Description

      There are multiple issues with Moodle files containing empty or blank lines[1] and/or white space before or after the PHP open and close tags, notably with config.php and theme files. ([1] Lines consisting of only white spaces or tabs.)

      So I've decided to search them all and request the developers to nuke them smile. As there are tons of files, I've come up with the following find + Perl one-liner that does all the dirty job automagicaly (run it at the top of your CVS working copy directory):

      find . -type f -exec perl -e '$/ = undef; $_ = <>; print $ARGV . "\n" if ((m#^([ t\n])<?php#si) or (m#?>\n([ \t\n])$#s) or (m#?>([ \t]+)$#s))' {} ";"

      It prints the paths of the offending files. As of today, there are 35 files having this 'problem'.

      If there is no objection to this, I can do it myself, but I'd want to get '+1' from some core developers first.

      Saludos. Iñaki.

        Gliffy Diagrams

          Attachments

            Activity

            Hide
            iarenaza Iñaki Arenaza added a comment -

            Adding some developers to request their opinion.

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - Adding some developers to request their opinion. Saludos. Iñaki.
            Hide
            dougiamas Martin Dougiamas added a comment -

            Yes yes yes! Thank you thank you thank you!

            I reckon this might be the source of a wierd user image problem I couldn't trace on moodle.org.

            Show
            dougiamas Martin Dougiamas added a comment - Yes yes yes! Thank you thank you thank you! I reckon this might be the source of a wierd user image problem I couldn't trace on moodle.org.
            Hide
            skodak Petr Skoda added a comment -

            my +1 too

            I am not sure about adodb stuff, should we report it upstream? Eloy?

            Show
            skodak Petr Skoda added a comment - my +1 too I am not sure about adodb stuff, should we report it upstream? Eloy?
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Has been reported here: http://phplens.com/lens/lensforum/msgs.php?id=16991

            +1 too (if we modify our adodb don't forget to add that to the readme_moodle.txt file to be able to track local modifications).

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Has been reported here: http://phplens.com/lens/lensforum/msgs.php?id=16991 +1 too (if we modify our adodb don't forget to add that to the readme_moodle.txt file to be able to track local modifications). Ciao
            Hide
            iarenaza Iñaki Arenaza added a comment -

            Fixed in CVS and modified lib/adodb/readme_moodle.txt as per Eloy's request

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - Fixed in CVS and modified lib/adodb/readme_moodle.txt as per Eloy's request Saludos. Iñaki.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Great job, Iñaki!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Great job, Iñaki!
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Hehe, only one small "but":

            Just noticed that you've applied the change to other branches but, have you moved the MOODLE_17_MERGED and MOODLE_18_MERGED floating tags?

            I guess no:

            (I've kept them separated from the official list)

            Ciao

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Hehe, only one small "but": Just noticed that you've applied the change to other branches but, have you moved the MOODLE_17_MERGED and MOODLE_18_MERGED floating tags? I guess no: http://docs.moodle.org/en/Development:Unmerged_files#MOODLE_17_-_MDL-11632 http://docs.moodle.org/en/Development:Unmerged_files#MOODLE_18_-_MDL-11632 (I've kept them separated from the official list) Ciao
            Hide
            iarenaza Iñaki Arenaza added a comment -

            Yeah, Eloy. I haven't forgot to update the tags. But switching branchs in CVS (at least with Sourceforge's CVS) is sooooooo painfully slow (it takes 20-30 minutes from home, where I do these things) and it was late at night. So I decided to leave it for today. I'm doing it right now

            Having used git for several months where switching Moodle branches takes 1-2 seconds, the CVS switching time feels like an eternity to me.

            Saludos. Iñaki.

            Show
            iarenaza Iñaki Arenaza added a comment - Yeah, Eloy. I haven't forgot to update the tags. But switching branchs in CVS (at least with Sourceforge's CVS) is sooooooo painfully slow (it takes 20-30 minutes from home, where I do these things) and it was late at night. So I decided to leave it for today. I'm doing it right now Having used git for several months where switching Moodle branches takes 1-2 seconds, the CVS switching time feels like an eternity to me. Saludos. Iñaki.
            Hide
            stronk7 Eloy Lafuente (stronk7) added a comment -

            Peeerfect!!

            Show
            stronk7 Eloy Lafuente (stronk7) added a comment - Peeerfect!!
            Hide
            dougiamas Martin Dougiamas added a comment -

            Hi Iñaki,

            Yes, it would be slow to do a complete switch in-place like that.

            Just a tip, it's better to just have a different checkout of each branch. I have /moodle/18, /moodle/19 directories for example. Disk is cheap.

            Show
            dougiamas Martin Dougiamas added a comment - Hi Iñaki, Yes, it would be slow to do a complete switch in-place like that. Just a tip, it's better to just have a different checkout of each branch. I have /moodle/18, /moodle/19 directories for example. Disk is cheap.
            Hide
            dougiamas Martin Dougiamas added a comment -

            And thanks very much for fixing this. Unfortunately it had no effect on my moodle.org profile images problem though. Doh!

            Show
            dougiamas Martin Dougiamas added a comment - And thanks very much for fixing this. Unfortunately it had no effect on my moodle.org profile images problem though. Doh!

              People

              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved:
                  Fix Release Date:
                  11/Oct/07