HeadTitle renderTitle returns rendered title without title tags #3877

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

matuszeman commented Feb 23, 2013

No description provided.

weierophinney was assigned Mar 8, 2013

Contributor

matuszeman commented Mar 11, 2013

@weierophinney hi.. will this be merged at some point?
I'm not sure why it says "failed". Do you have any idea so I can fix it?

Owner

weierophinney commented Mar 11, 2013

@matuszeman Click the "Details" link to see the failure; in this case, it's a transient timing one, so nothing to be worried about.

I don't see the purpose of the change you're introducing. Under the intended usage, you will call either $helper->toString() or echo $helper (the latter is usually done as echo $this->headTitle()) in order to get the <title>...</title> string. That functionality is well tested (it has existed since v1.5.0 of ZF).

Unless you can clarify what's not working by means of a unit test, I'm inclined to close this PR.

Contributor

matuszeman commented Mar 12, 2013

The purpose of this is to get title string without <title> tag - so you can log it, send it back to the browser via ajax json, etc...

Owner

weierophinney commented Mar 12, 2013

Then write up some unit tests. :-)

On Monday, March 11, 2013, Matus Zeman wrote:

The purpose of this is to get title string without <title> tag - so you
can log it, send it back to the browser via ajax json, etc...


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/3877#issuecomment-14752296
.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

Contributor

matuszeman commented Mar 12, 2013

I will do so later today. You never know if PR is going to be accepted or not that's why I'm really careful with my time spent while contributing to ZF2.

Owner

weierophinney commented Mar 12, 2013

@matuszeman Actually, unit tests are probably the better way to spend time when creating a PR, as they show us the expectations you have, and demonstrate the use case. Basically, they help answer questions we might otherwise ask. :)

Owner

weierophinney commented Mar 25, 2013

@matuszeman Any word on tests? I don't want to keep it open too long and have it go out-of-date with master.

Thanks!

Contributor

matuszeman commented Mar 27, 2013

Sorry ... I've been sick recently and trying to catch up now. I will deal with this asap.

Owner

weierophinney commented Apr 11, 2013

@matuszeman Any word on tests? If you won't be able to complete soon, I'll close, and you can re-submit when you have them ready.

@weierophinney weierophinney added a commit that referenced this pull request Apr 15, 2013

@weierophinney weierophinney [#3877] CS fixes
- per php-cs-fixer
b97dc35

@weierophinney weierophinney added a commit that referenced this pull request Apr 15, 2013

@weierophinney weierophinney Merge branch 'hotfix/3877' into develop
Close #3877
71f4f9f
Owner

weierophinney commented Apr 15, 2013

Merged to develop for release with 2.2.0, as this contains a new method.

Also, in the future, please create a new branch per pull request. Since you did this against your master branch, it also contained an unrelated change to the router; I had to cherry-pick the individual commits related to this pull request, which rewrites history (though your authorship is retained).

@weierophinney weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015

@weierophinney weierophinney [zendframework/zendframework#3877] CS fixes
- per php-cs-fixer
2ee00a3

@weierophinney weierophinney added a commit to zendframework/zend-view that referenced this pull request May 15, 2015

@weierophinney weierophinney Merge branch 'hotfix/3877' into develop 80332f8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment