Use an autoformatter for the indentation. #481

Merged
merged 2 commits into from Nov 6, 2011

Projects

None yet

5 participants

@realityking
Joomla! member

No description provided.

@joomla-jenkins

Unit testing complete. There were 0 failures and 0 errors from 1683 tests and 10559 assertions.
Checkstyle analysis reported 453 warnings and 1 errors.

@elkuku elkuku and 1 other commented on an outdated diff Nov 2, 2011
libraries/joomla/cache/controller/view.php
@@ -69,7 +69,7 @@ class JCacheControllerView extends JCacheController
}
else
- { // No workarounds, so all data is stored in one piece
+ { // No workarounds, so all data is stored in one piece
echo (isset($data)) ? $data : null;
@elkuku
elkuku Nov 2, 2011

The formatter failed :P - put the comment on a new line ?

@elinw
elinw Nov 4, 2011

Comments should definitely be on their own lines above the code they refer to but afaik the sniffs don't deal with that at all .... but there should no be a tab the { should align with the else.

@elkuku
elkuku Nov 4, 2011

Should be possible to create such a sniff ;)

@elinw
elinw Nov 4, 2011

I have a pretty detailed set of rules that I used in proof reading all of the comments last spring.

E.g. always a space after //
always start with capital unless it is something that is specifically lower case or a continuation of a sentence.
And so on.

@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/cache/storage/file.php
@@ -197,10 +197,10 @@ class JCacheStorageFile extends JCacheStorage
$folders = $this->_folders($this->_root);
for ($i = 0, $n = count($folders); $i < $n; $i++)
{
- if ($folders[$i] != $folder)
- {
- $return |= $this->_deleteFolder($this->_root . '/' . $folders[$i]);
- }
+ if ($folders[$i] != $folder)
+ {
+ $return |= $this->_deleteFolder($this->_root . '/' . $folders[$i]);
+ }
}
@realityking

These are fails too. But at least I got an Idea what caused these ;)

@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/filesystem/file.php
@@ -492,7 +492,7 @@ class JFile
else
{
if (is_writeable($baseDir) && move_uploaded_file($src, $dest))
- { // Short circuit to prevent file permission errors
+ { // Short circuit to prevent file permission errors
if (JPath::setPermissions($dest))
@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/filesystem/stream.php
case 'bz':
$this->_fh = bzopen($filename, $mode);
break;
- // fopen can handle streams
+ // fopen can handle streams
case 'f':
@elkuku
elkuku Nov 2, 2011

those 3 look strange..

@elkuku elkuku and 1 other commented on an outdated diff Nov 2, 2011
libraries/joomla/form/fields/groupedlist.php
@@ -117,7 +117,7 @@ class JFormFieldGroupedList extends JFormField
}
break;
- // Unknown element type.
+ // Unknown element type.
default:
@elinw
elinw Nov 4, 2011

That comment should not have a tab, it should be aligned with the code it refers to.

@elkuku elkuku commented on the diff Nov 2, 2011
libraries/joomla/form/form.php
@@ -1311,7 +1311,7 @@ class JForm
break;
default:
- // Check for a callback filter.
+ // Check for a callback filter.
if (strpos($filter, '::') !== false && is_callable(explode('::', $filter)))
@realityking
Joomla! member

Good comments. I see what I can fix in the autoformatter.

@elkuku elkuku and 1 other commented on an outdated diff Nov 2, 2011
libraries/joomla/form/rules/url.php
@@ -75,7 +75,7 @@ class JFormRuleUrl extends JFormRule
}
// For some schemes here must be two slashes.
if (($urlScheme == 'http' || $urlScheme == 'https' || $urlScheme == 'ftp' || $urlScheme == 'sftp' || $urlScheme == 'gopher'
- || $urlScheme == 'wais' || $urlScheme == 'gopher' || $urlScheme == 'prospero' || $urlScheme == 'telnet')
+ || $urlScheme == 'wais' || $urlScheme == 'gopher' || $urlScheme == 'prospero' || $urlScheme == 'telnet')
&& ((substr($value, strlen($urlScheme), 3)) !== '://'))
@elinw
elinw Nov 4, 2011

Definitely should not be a tab there.

@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/installer/adapters/template.php
@@ -131,7 +131,7 @@ class JInstallerTemplate extends JAdapterInstance
$this->parent->setOverwrite(true);
$this->parent->setUpgrade(true);
if ($id)
- { // if there is a matching extension mark this as an update; semantics really
+ { // if there is a matching extension mark this as an update; semantics really
$this->route = 'update';
@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/string/string.php
@@ -819,7 +819,7 @@ abstract class JString
|| (4 < $mBytes)
|| (($mUcs4 & 0xFFFFF800) == 0xD800) // From Unicode 3.2, surrogate characters are illegal
|| ($mUcs4 > 0x10FFFF)) // Codepoints outside the Unicode range are illegal
- {
+ {
return false;
@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/updater/adapters/extension.php
@@ -44,7 +44,7 @@ class JUpdaterExtension extends JUpdateAdapter
$this->current_update->update_site_id = $this->_update_site_id;
$this->current_update->detailsurl = $this->_url;
break;
- // Don't do anything
+ // Don't do anything
case 'UPDATES':
@elkuku elkuku commented on the diff Nov 2, 2011
libraries/joomla/updater/adapters/extension.php
@@ -100,7 +100,7 @@ class JUpdaterExtension extends JUpdateAdapter
}
break;
case 'UPDATES':
- // :D
+ // :D
break;
@elkuku elkuku commented on the diff Nov 2, 2011
libraries/joomla/updater/update.php
@@ -228,7 +228,7 @@ class JUpdate extends JObject
}
break;
case 'UPDATES':
- // If the latest item is set then we transfer it to where we want to
+ // If the latest item is set then we transfer it to where we want to
if (isset($this->_latest))
@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/user/helper.php
- {
- $new .= $plaintext;
- }
- $new .= ($i & 1) ? substr($binary, 0, 16) : $plaintext;
- $binary = JUserHelper::_bin(md5($new));
+ $new = ($i & 1) ? $plaintext : substr($binary, 0, 16);
+ if ($i % 3)
+ {
+ $new .= $salt;
+ }
+ if ($i % 7)
+ {
+ $new .= $plaintext;
+ }
+ $new .= ($i & 1) ? substr($binary, 0, 16) : $plaintext;
+ $binary = JUserHelper::_bin(md5($new));
}
@elkuku
elkuku Nov 2, 2011

this produces 2 tabs instead of one.. ?

@elkuku elkuku commented on an outdated diff Nov 2, 2011
libraries/joomla/utilities/utility.php
@@ -238,7 +238,7 @@ class JUtility
switch ($last)
{
- // The 'G' modifier is available since PHP 5.1.0
+ // The 'G' modifier is available since PHP 5.1.0
case 'g':
@elkuku
elkuku Nov 2, 2011

and a final fail

@elkuku
Joomla! member

Overall, very nice. Don't trust the auto formatters :P
sorry for the spam :(

@realityking
Joomla! member

This should fix all your comments. I probably should fix as much as possible in the autoformatter too, but who has the time ;)

@joomla-jenkins

Build triggered by changes to the head.

Unit testing complete. There were 0 failures and 0 errors from 1742 tests and 10618 assertions.
Checkstyle analysis reported 453 warnings and 29 errors.

@joomla-jenkins

Build triggered by changes to the base.

The tests completed but there was a problem parsing the report.
Unit testing complete. There were 0 failures and 0 errors from 1742 tests and 10618 assertions.
Checkstyle analysis reported 453 warnings and 29 errors.

@joomla-jenkins

Build triggered by changes to the base.

The tests completed but there was a problem parsing the report.
Unit testing complete. There were 0 failures and 0 errors from 1683 tests and 10559 assertions.
Checkstyle analysis reported 441 warnings and 0 errors.

@joomla-jenkins

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 441 warnings and 19 errors.

@joomla-jenkins

Build triggered by changes to the base.

Test log missing. Tests failed to execute.
Checkstyle analysis reported 441 warnings and 19 errors.

@eddieajau eddieajau merged commit e2592f8 into joomla:staging Nov 6, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment