Skip to content

Conversation

@frankmayer
Copy link
Contributor

@frankmayer frankmayer commented Dec 19, 2016

Summary of Changes

  • Type safe string comparison of strings only, in libraries/cms

This PR is part of a set to try to separate some of the changes done in one of my previous batch PR's for the libraries/cms directory, which is still on hold (#12171).
Once the new set is merged completely, it will hopefully reduce the changes in that PR, so it can be reviewed easier and finally be merged.

The changes in this PR should be very easy to review, as they are only string and string type comparisons. In hope that this will get merged quickly. ;)

Note: Don't bother if some possible changes are missing or could be differently written. They are probably in the batch PR , that this one references. As soon as this set of sub PR's is merged, the batch PR will have its conflicts resolved and should be a lot easier to review and finally get merged.

Testing Instructions

None, should not change behavior

Documentation Changes Required

None.

@frankmayer frankmayer changed the title Type safety in libraries cms Type safe comparison of strings only, in libraries/cms Dec 22, 2016
@frankmayer frankmayer mentioned this pull request Jan 5, 2017
6 tasks
# Conflicts:
#	libraries/cms/installer/installer.php
#	libraries/cms/schema/changeitem.php
#	libraries/cms/schema/changeset.php
@frankmayer
Copy link
Contributor Author

Conflicts resolved...

# Conflicts:
#	libraries/cms/component/router/rules/standard.php
@frankmayer
Copy link
Contributor Author

Conflicts resolved. Pls check and merge.


// If gzip compression is enabled in configuration and the server is compliant, compress the output.
if ($this->get('gzip') && !ini_get('zlib.output_compression') && (ini_get('output_handler') != 'ob_gzhandler'))
if ($this->get('gzip') && !ini_get('zlib.output_compression') && (ini_get('output_handler') !== 'ob_gzhandler'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

$file = $this->input->get('tmpl', 'index');

if (!$this->get('offline') && ($file == 'offline'))
if (!$this->get('offline') && ($file === 'offline'))
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses


// Add an option to show all?
$showAll = isset($this->element['showall']) ? ($this->element['showall'] == 'true') : true;
$showAll = isset($this->element['showall']) ? ($this->element['showall'] === 'true') : true;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

{
// If mode="nested" || ( mode not set & config = nested )
if ((isset($this->element['mode']) && $this->element['mode'] == 'nested')
if ((isset($this->element['mode']) && $this->element['mode'] === 'nested')
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

else
{
$direction = ($direction == 'desc') ? 'asc' : 'desc';
$direction = ($direction === 'desc') ? 'asc' : 'desc';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

else
{
$direction = ($direction == 'desc') ? 'asc' : 'desc';
$direction = ($direction === 'desc') ? 'asc' : 'desc';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

$this->extension->state = 0;
$this->extension->name = $manifest_details['name'];
$this->extension->enabled = ('editors' == $this->extension->folder) ? 1 : 0;
$this->extension->enabled = ('editors' === $this->extension->folder) ? 1 : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

foreach ($element->children() as $file)
{
$fCharset = (strtolower($file->attributes()->charset) == 'utf8') ? 'utf8' : '';
$fCharset = (strtolower($file->attributes()->charset) === 'utf8') ? 'utf8' : '';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses


// Is this path a file or folder?
$path['type'] = ($file->getName() == 'folder') ? 'folder' : 'file';
$path['type'] = ($file->getName() === 'folder') ? 'folder' : 'file';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses


// Is this path a file or folder?
$path['type'] = ($file->getName() == 'folder') ? 'folder' : 'file';
$path['type'] = ($file->getName() === 'folder') ? 'folder' : 'file';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

// Get the template and file name from the string
$temp = explode(':', $layout);
$template = ($temp[0] == '_') ? $template : $temp[0];
$template = ($temp[0] === '_') ? $template : $temp[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

// Get the template and file name from the string
$temp = explode(':', $layout);
$template = ($temp[0] == '_') ? $template : $temp[0];
$template = ($temp[0] === '_') ? $template : $temp[0];
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

$template = ($temp[0] == '_') ? $template : $temp[0];
$template = ($temp[0] === '_') ? $template : $temp[0];
$layout = $temp[1];
$defaultLayout = ($temp[1]) ? $temp[1] : 'default';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

$template = ($temp[0] == '_') ? $template : $temp[0];
$template = ($temp[0] === '_') ? $template : $temp[0];
$layout = $temp[1];
$defaultLayout = ($temp[1]) ? $temp[1] : 'default';
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove parentheses

@Quy
Copy link
Contributor

Quy commented May 28, 2017

I have tested this item ✅ successfully on bae19d7
Code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13275.

@andrepereiradasilva
Copy link
Contributor

I have tested this item ✅ successfully on bae19d7

on code review


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/13275.

@ghost
Copy link

ghost commented May 31, 2017

RTC after two successful tests.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label May 31, 2017
@rdeutz rdeutz merged commit 5aa724a into joomla:staging Jun 13, 2017
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 13, 2017
@rdeutz rdeutz added this to the Joomla 3.7.3 milestone Jun 13, 2017
@frankmayer frankmayer deleted the type-safety-in-libraries-cms branch June 13, 2017 21:57
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.

5 participants