add thematic_show_commentreply to wp_enqueue_scripts instead of calling in header.php

[GoogleCodeExporter]

similar to issue 137 
http://code.google.com/p/thematic/issues/detail?id=137

// Enables comment threading
thematic_show_commentreply()

in header.php is unnecessary (i see it in the todo)

function thematic_show_commentreply() {
    $display = TRUE;
    $display = apply_filters('thematic_show_commentreply', $display);
    if ($display)
        if ( is_singular() ) 
            wp_enqueue_script('comment-reply'); // support for comment threading
}
add_action('wp_enqueue_scripts','thematic_show_commentreply');

i'll try to make a patch for it.  

Original issue reported on code.google.com by helgathe...@gmail.com on 26 Jan 2012 at 10:45

[GoogleCodeExporter]

Great. I think the latest recommendation I saw regarding the inclusion 
conditionals was

if ( is_singular() && comments_open() && get_option( 'thread_comments' ) )
    wp_enqueue_script( 'comment-reply' );
}

Original comment by invistr...@gmail.com on 26 Jan 2012 at 11:05

[GoogleCodeExporter]

that is how i am seeing it in other themes.  

Original comment by helgathe...@gmail.com on 26 Jan 2012 at 11:12

[GoogleCodeExporter]

wondering where to put it if thematic_show_commentreply() is removed from 
header.php ?  best to still leave in header-extensions.php?  

twentyeleven has it hardcoded in header.php like so:

if ( is_singular() && get_option( 'thread_comments' ) )
        wp_enqueue_script( 'comment-reply' );

Original comment by helgathe...@gmail.com on 26 Jan 2012 at 11:17

[GoogleCodeExporter]

Really? Goes to show that not even they are following all of the 
recommendations. Or the recommendation to put everything in hooks is new enough 
that we might first see it in twentytweleve when it comes out.

I would put it in thematic_head_scripts() along with all the other scripts.

Original comment by invistr...@gmail.com on 27 Jan 2012 at 9:57

[GoogleCodeExporter]

Like this

Original comment by invistr...@gmail.com on 27 Jan 2012 at 11:01

Attachments:

• -138-enqueue_commentreply.diff

[GoogleCodeExporter]

seems like a logical place, so looks ok to me.  though i do want all those init 
functions moved to functions.php.  

Original comment by helgathe...@gmail.com on 27 Jan 2012 at 2:52

[GoogleCodeExporter]

I think that enqueuing by hook( as you don't hook before init ) or adding 
directly in the header are both acceptable practices.
The Codex still recommends to include the enqueuing script in the header just 
before wp_head()

http://codex.wordpress.org/Migrating_Plugins_and_Themes_to_2.7/Enhanced_Comment_
Display#Javascript_Comment_Functionality

 Theme Review sets a higher standard and thats fine; but we need to choose our battles wisely.

I don't think this is simple as applying that patch and moving on. These are 
other considerations to make like deprecation of functions and what to do with 
the filters inside them.

If were going to get 0.9.8 released soon we need to throttle back on this and 
look to future milestones for what were discussing here.

There will be room for exceptions to the review guidelines considering we're 
"Previously Reviewed" .  Also we can make the point to the reviewer before hand 
that the main point of this to release is securing the theme options script 
which should be paramount over the little details like what script gets 
enqueued where. 

Original comment by eugene.m...@gmail.com on 27 Jan 2012 at 4:44

• Changed state: Accepted

[GoogleCodeExporter]

Removing direct calls to enqueued items will affect both 
thematic_show_commentreply() and thematic_create_stylesheet()

If were going to do away with those functions they need to be deprecated to 
inform users(of child themes) before possibly causing a fatal error. 

Use of the filter thematic_show_commentreply will need to be addressed. Maybe 
something like this would work:

    if ( has_filter('thematic_show_commentreply') )
        _doing_it_wrong( __FUNCTION__, sprintf( __( 'The %1$s filter is deprecated. Please use %2$s' ), 'thematic_show_commentreply', '<code>wp_dequeue_script(\'comment-reply\')</code>' ), '0.9.8' ); 

Original comment by eugene.m...@gmail.com on 27 Jan 2012 at 4:47

[GoogleCodeExporter]

I forgot the text domain; but I think the localization string is general enough 
to apply to deprecation of more filters down the road. 

If _deprected_hook ever makes it into WP 
http://core.trac.wordpress.org/ticket/10441 __doing_it_wrong won't be needed

Original comment by eugene.m...@gmail.com on 27 Jan 2012 at 4:56

[GoogleCodeExporter]

Had time to think it over. No worries on the stylesheet... my misinterpretation 
of that guideline. Using _doing_it_wrong would be doing it wrong lol. 

Look this patch over and see what you think.

Original comment by eugene.m...@gmail.com on 27 Jan 2012 at 7:55

• Changed state: Started

Attachments:

• 138_commentreply.diff

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

After all that we have r817

* Moved: enqueue of comment repy scipt to <code>thematic_head_scripts()</code>
* Deprecated: thematic_show_commentreply;
* Replaced: <code>get_bloginfo(*)</code> for Theme Review required functions
* Moved: <code>thematic_create_contenttype()</code> within header.php for Theme 
Review compliance
* Removed: <code>wp_register_*</code> because <code>wp_enqueue_*</code> 
registers * when the src param is set for <code>_scripts</code> and 
<code>_styles</code>
* Moved: Function <code>thematic_head_scripts</code> to header.php
* Changed: the action hook for <code>thematic_head_scripts()</code> and 
<code>thematic_create_stylesheet()</code> to <code>wp_enqueue_scripts</code>

Original comment by eugene.m...@gmail.com on 30 Jan 2012 at 12:25

• Changed state: Fixed

[GoogleCodeExporter]

Please review to verify fix.

Original comment by eugene.m...@gmail.com on 30 Jan 2012 at 12:29

[GoogleCodeExporter]

hey gene, i'll check this revision out maybe tomorrow... but i'm also moving 
again on tuesday so it might have to wait until wednesday or so

Original comment by helgathe...@gmail.com on 30 Jan 2012 at 4:03