Skip to content

Update libraries/joomla/html/html.php #1726

Merged
merged 4 commits into from Mar 16, 2013

4 participants

@e-motiv
e-motiv commented Nov 30, 2012

Did several things here:

  • Fixed the wrong statements about the function's parameter's in the comments
  • Uses ternary operator for pathonly return (wee bit less and better(?) code)
  • Allow faster result if you just want the tag without looking for possible alternatives -> by re-using path_only since it does not make sense asking only the path without computing
  • We don't want to check the attribs if path_only is required, hence putting it in the bottom if; and made ternary operation of it
  • reuse $file, 2 (potential) reasons: 1. wee bit less of code (why not?) 2. You can use this function as check for $file and changing the variable itself to the right path (if it was right or not). That way the user of the function will have a variable less (why not?)
@e-motiv e-motiv Update libraries/joomla/html/html.php
Did several things here:
* Fixed the wrong statements about the function's parameter's in the comments
* Uses ternary operator for pathonly return (wee bit less and better(?) code)
* Allow faster result if you just want the tag without looking for possible alternatives 
	-> by re-using path_only since it does not make sense asking only the path without computing
* We don't want to check the attribs if path_only is required, hence putting it in the bottom if; and made ternary operation of it
* reuse $file, 2 (potential) reasons: 1. wee bit less of code (why not?) 2. You can use this function as check for $file and changing the variable itself to the right path (if it was right or not). That way the user of the function will have a variable less (why not?)
63e8e18
@elinw
elinw commented Dec 2, 2012

Hi @e-motiv
You have some code style errors:
http://developer.joomla.org/pulls/pulls/1726.html

@e-motiv
e-motiv commented Dec 4, 2012

Hello elinw,

I know. hple (from joomla irc) already warned me about that.
But I can't find the button or link where to change those errors on my pull request.
I am working via browser on GitHub and though I guess it has to do with this sentence below here : "You can add more commits to this pull request by pushing to the patch-1 branch on e-motiv/joomla-platform", I can't find anywhere how to do that. I've searched for a half an hour..
Or is there another way (via browser)?
(I know eclipse has a Git plugin, but I don't need/want a full git sync there now.)

Also, if anyone happens to know a file I can import in eclipse to set my code style to joomla requirements, that would save a lot of manual time too.

@elinw
elinw commented Dec 4, 2012

Go to your own repository and on the drop down select the branch that you are using which is patch-1. Then go to the file you need to fix and click on edit.

https://github.com/e-motiv/joomla-platform/tree/patch-1

@e-motiv
e-motiv commented Jan 30, 2013

Thanks for your help @elinw. I fixed those meanwhile. Is there anything else or can this be committed now?

I am also wondering if this gets committed, will it be done for all future versions? I am sure it would then get in the latest joomla version (3.x or what will it be), but does this also get update in an older update version, like any 2.5 future update?

@e-motiv e-motiv closed this Jan 30, 2013
@e-motiv e-motiv reopened this Jan 30, 2013
@ianmacl ianmacl and 1 other commented on an outdated diff Jan 30, 2013
libraries/joomla/html/html.php
}
else
{
- return '<img src="' . (count($includes) ? $includes[0] : '') . '" alt="' . $alt . '" ' . $attribs . ' />';
+ return '<img src="' . $file . '" alt="' . $alt . '" ' .
+ (is_array($attribs)?JArrayHelper::toString($attribs):$attribs) .
@ianmacl
ianmacl added a note Jan 30, 2013

Not sure why that isn't flagged as a coding style violation. Please make it:
(is_array($attribs) ? JArrayHelper::toString($attribs) : $attribs)

Other than that it looks good and we'lll get it merged in.

@e-motiv
e-motiv added a note Jan 31, 2013

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@eddieajau eddieajau merged commit 6cb5d4b into joomla:staging Mar 16, 2013
@e-motiv e-motiv deleted the e-motiv:patch-1 branch Apr 23, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.