Clone input nodes when inserting over a set

[jugglinmike]

I noticed this bug while wrapping up gh-558 (c'mon that's funny). I think the changeset ended up being a little ugly, so please don't hesitate to offer suggestions for refactoring!

Commit message:

This patch increases parity with jQuery, specifically in cases where
manipulation methods are called with existing nodes on sets containing
more than one element. In such cases, the provided node should be cloned
prior to insertion (excepting the final element in the set, where the
original node should be inserted).

[coveralls]

Coverage increased (+0.02%) when pulling 7772bf6 on jugglinmike:clone-set into 6ba5338 on cheeriojs:master.

[davidchambers]

We could replace str.length >= 3 with str !== '<>'.

[jugglinmike]

We could, but I'm not sure that would be an improvement since it duplicates information. The preceeding conditions already checked for the character values, after all.

If you're motivated by optimization, I wouldn't necessarily object (this is used on pretty hot code path, after all). In that case, though, we'll have to microbenchmark and consider a bunch of other alternatives (str.length > 2 comes to mind, along with comparing the relative speed of charAt and the array subscripting operator).

[coveralls]

Coverage increased (+0.02%) when pulling 8340187 on jugglinmike:clone-set into 6ba5338 on cheeriojs:master.

[jugglinmike]

@davidchambers I just realized that it was coimpletely unecessary to move the isHtml function for this pull request. I've undone that change in an additional commit, which I will squash at your command. I've also opened gh-578 to discuss the issues we've identified around input interpretation/validation.

(In case you're curious: I noticed this bug while working on another feature branch. I cherry-picked that commit onto master and it looks like some extra changes came along for the ride.)

[davidchambers]

In such cases, the provided node should be cloned prior to insertion (excepting the final element in the set, where the original node should be inserted).

Why is the last element in a set treated differently?

[jugglinmike]

@davidchambers I want to match jQuery's behavior. From the API documentation on append:

Important: If there is more than one target element, however, cloned copies of the inserted element will be created for each target except for the last one.

I can only guess at the rationale. Here goes:

When calling, for instance, $div.append($existingElement), one would expect the $existingElement to be removed from its original location and appended to the element in the$div selection. Identical behavior would not be possible if append were called on $divs--a collection of elements. My guess is that the behavior is an attempt to maintain both behaviors (content insertion and content movement) in cases where multiple target elements are specified.

Again, this is completely conjecture on my part, since the API documentation doesn't really cover motivation for design decisions. What do you think?

[davidchambers]

That makes sense, @jugglinmike! Thanks for the explanation. :)

[fb55]

Needs a rebase, LGTM, though.

[coveralls]

Coverage increased (+0.02%) when pulling 56fbefb on jugglinmike:clone-set into d05ce4c on cheeriojs:master.

[coveralls]

Coverage increased (+0.02%) when pulling 466e34a on jugglinmike:clone-set into 3b4fb1e on cheeriojs:master.

[jugglinmike]

@fb55 I've updated this one, too

[fb55]

And merged.

[jugglinmike]

Yessss

[fb55]

:D