[#6684] - Allow jdoc tags to be written with closing tags
- Closed
- 12 Apr 2016
- Medium
- Build: staging
- Easy Test
- # 6684
- Diff
- okonomiyaki3000:jdoc-tag-improvement
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
What?
Typical JDoc tags are self closing, like this:
<jdoc:include blahblahblah />With this change, that will still work but now so will this:
<jdoc:include blahblahblah></jdoc:include>Why?
Practically, it makes no difference but why shouldn't we be able to write these tags any way we like?
OK, there are a couple of nice things about using a closing tag.
1) If your IDE is 'helping' you by closing your tags, it might not recognize that <jdoc:include /> is closed because HTML has a finite number of self-closing tags and <jdoc:include /> is not among them. Therefore, an IDE that knows HTML well will probably see this tag as still open and try to help you close it.
2) It's a bit silly but, you could do something like this:
<jdoc:include type="modules" name="foo">
This is the 'foo' module space, it will usually be used with 'bar' modules but sometimes may include 'baz' modules.
</jdoc:include>The text inside the tag will essentially be a comment that will be miraculously stripped away when the template output is parsed.
How (to test)?
Well, this is change to a single line of code. It's the line that parses <jdoc:include> tags in your template so to test the change:
1) Load up any page of your site that uses <jdoc:include /> (i.e. any page). If all of your modules, component, head, etc are loading, the existing functionality is not broken.
2) In your template file, change those <jdoc:include /> tags (some or all, it doesn't matter) to <jdoc:include></jdoc:include> and load the page again. Everything should continue to work exactly as usual. Then the new functionality is also not broken.
3) Just for good measure, change those tags to <jdoc:include /></jdoc:include> and load the page. Now you should see that everything appears where it's expected but, in the HTML source of the page there will be some </jdoc:include> left behind in those places. This expected because that's not the right way to write these tags. So then, the change is working as it should.
WTF?
I got some unit test failures with this change but I assure you, they are not related to this change. I will fix them in a different PR. Because they have nothing to do with this. At all.
joomla-cms-bot
- | Labels |
Added:
?
|
||
| Labels |
Added:
?
|
||
zero-24
- | Title |
|
||||||
| Easy | No | ⇒ | Yes | ||||
| Category | ⇒ | Libraries |
| Title |
|
||||||
| Title |
|
||||||
I just realized that in my original How to test section, something was messed up because I used the wrong quotes so any <> were stripped and nothing made any sense at all. It should be more understandable now.
Tested successfully. :-)
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6684.
Maybe I did something wrong. I applied the PR and
I changed this line in protostar
<jdoc:include type="head" />
to
<jdoc:include type="head">
And added
</jodc:include>
Before the closing head tag
And this broke the template
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6684.
You've got a typo here: "jodc" instead of "jdoc". Could that be it?
No the typo was just here on github
@brianteeman Could you clarify that earlier post, I guess something got stripped out of it so it's not clear what you changed. You may need to use < and > instead of < and >
Wrapped the tags in some markdown, should be good now.
OK, I just tried the same thing and it does break (the tag is not properly replaced). So I may need to fix something. But you'd never want to do it that way anyway. If you put </jdoc:include> just before </head> then you will be enclosing a lot of things (a link tag, some style tags, etc.) inside of the jdoc tag. All of those things would be stripped out when the jdoc tag is replaced. You should really just replace <jdoc:include type="head" /> with <jdoc:include type="head"></jdoc:include>
This should solve it.
It no longer completely breaks the template BUT it stops certain parts of the template from loading.
A quick look suggests to me that anything that is php between the <jdoc:include type="head"></jdoc:include> is simply ignored
Re-reading your comments this is what you expected to happen. In which case then its really not a good change as something doesnt work but there is no obvious reason for it not to work. Sorry but I think the usecase case for making this change is not worth the potential headaches
Well, what would you expect to happen to anything enclosed in the tag?
That it would work
On 18 Jun 2015 7:12 am, "Elijah Madden" notifications@github.com wrote:
Well, what would you expect to happen to anything enclosed in the tag?
—
Reply to this email directly or view it on GitHub
#6684 (comment).
Yeah, but what does that mean? The purpose of these tags is to be replaced by some other content so it's not really clear why you'd want them to enclose anything and, if they do, what you'd want to do with that content when the tag is replaced. Should the enclosed content go before or after the replacement content? Should the enclosed content be modified in any way?
Actually, by enclosing content like this, some interesting possibilities are available like maybe you'd have a <jdoc:debug></jdoc:debug> tag which contains content which will only be shown in debug mode or something like that.
I guess I just dont see the benefit then
On 18 June 2015 at 07:19, Elijah Madden notifications@github.com wrote:
Yeah, but what does that mean? The purpose of these tags is to be replaced
by some other content so it's not really clear why you'd want them to
enclose anything and, if they do, what you'd want to do with that content
when the tag is replaced. Should the enclosed content go before or after
the replacement content? Should the enclosed content be modified in any
way?Actually, by enclosing content like this, some interesting possibilities
are available like maybe you'd have a jdoc:debug/jdoc:debug tag which
contains content which will only be shown in debug mode or something like
that.—
Reply to this email directly or view it on GitHub
#6684 (comment).
Brian Teeman
Co-founder Joomla! and OpenSourceMatters Inc.
http://brian.teeman.net/
Imho, it's a tag which isn't supposed to contain anything, so why should it be allowed to have a closing tag? It would just confuse people because it implies that you can put something useful between the opening and closing tag.
If you need to add comments, use either PHP or HTML comments. There is no need to invent a new comment format 
As for IDE support, I would love to see that for example PhpStorm would recognize our jdoc namespace, however this PR doesn't help with it. PhpStorm does close it fine with /> as soon as you enter /. However it adds the closing tag if you end the jdoc tag with > alone. So just write it right and it works fine.
So I really don't see a point with this PR. But maybe I miss the important part?
The ability for it serve as comments is a side effect not really the main purpose. There could be other uses for the enclosed content, if any, but I'm not proposing any at this time.
There are many cases in which you'd have a tag and closing tag with no content. It's a common occurrence with the <script> tag, for example. So this is not unusual.
The main issue that I'm addressing is a minor one and has to do with certain IDEs. Certainly SublimeText, probably others. If this tag is recognized as self-closing (void element) by PhpStorm, then it is because PhpStorm is only observing a pattern rather than adhering to the actual HTML spec. The spec defines a finite list of such tags (16 of them). An IDE that is aware of this fact will not recognize any other tag as self-closing even if you have a / at the end. Then it will try to 'help' you by suggesting to close this tag whenever you type </ or, depending on your settings, it may just create the closing tag immediately when you write the tag in the first place.
So, yeah, this is not particularly important it just seemed logical that this tag should be able to be written in either form.
I am setting this to needs review so the cms maintainers can decide
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6684.
I am setting this to needs review so the cms maintainers can decide
This comment was created with the J!Tracker Application at issues.joomla.org/joomla-cms/6684.
| Status | Pending | ⇒ | Needs Review |
This testing failure looks like travis' problem.
I'm closing this. I don't think we want people to treat these things as XML tags - they aren't - and the distinction is that colons in XML tags have a special namespace meaning - which is not what is intended in the template use case. I also think that this will likely confuse more people than it helps as evidenced in this PR.
| Status | Needs Review | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2016-04-12 23:51:19 |
| Closed_By | ⇒ | wilsonge |
Well, if it's OK with Travis, it's OK with me but when I run the tests locally I get an error because
JFactory::getApplication()is called without any id. Looks like if I run the tests on the entire libraries folder, that doesn't happen but it's broken if I try to run on just a specific folder. It's pretty obvious why that happens but it shouldn't be possible.