Skip to content

Conversation

@markelog
Copy link
Member

@markelog markelog commented Jan 7, 2013

Now, when manipulation module was reduced, maybe it's time to refactor some stuff?

You removed jQuery.buildFragment, but it seems jQuery.clean is one that should be removed. If this changes is acceptable i could ported it to 1.9 too.

@markelog
Copy link
Member Author

markelog commented Jan 7, 2013

In migrate plugin, jQuery.clean could be implemented like that –

jQuery.clean = function( elems, context, fragment, scripts, selection ) {
    var newFragment = jQuery.buildFragment( elems, context || document, scripts, selection );

    if ( fragment ) {
        fragment.appendChild( newFragment );

    } else {
        fragment = newFragment;
    }

    return jQuery.merge( [], fragment.childNodes );
};

But i did not test this code thoroughly

Copy link
Member

Choose a reason for hiding this comment

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

Both this and L471 are prime candidates for our i = 0; while ( (elem = array[i++]) ) { pattern, but failing that I'd still check the compressibility of putting initial assignments before the for.

Copy link
Member Author

Choose a reason for hiding this comment

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

done, it saves 15 bytes or so and it definitely looks more elegant.

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you use also core_push here? Or maybe push instead of core_push in L446?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch!

@dmethvin
Copy link
Member

dmethvin commented Jan 8, 2013

2.0: Landed in unsquashed commits ending at 57d9dcd.

1.9: Similar edits landed at 0ed497b -- @Orkel could you take a look and see if I got that right? It can probably use some cleanup but I wanted to land it asap.

@dmethvin dmethvin closed this Jan 8, 2013
gibson042 pushed a commit that referenced this pull request Jan 9, 2013
mescoda pushed a commit to mescoda/jquery that referenced this pull request Nov 4, 2014
mescoda pushed a commit to mescoda/jquery that referenced this pull request Nov 4, 2014
@lock lock bot locked as resolved and limited conversation to collaborators Jan 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants