Ability to specify index when copying

[mattsgarlata]

The move function allows supplying an index so you can move a node to a particular spot in the tree. I also need to be able to do that when copying for https://github.com/SpiderStrategies/Scoreboard/issues/30102

[mattsgarlata]

@nathanbowser before you get started on this, let me discuss with Scott and make sure we want to tackle this. I'm 90% sure we need it, but let me make sure

[nathanbowser]

@mattsgarlata Sounds good. Please let me know if it should be top priority, too.

[nathanbowser]

@mattsgarlata Would like your feedback and preference on the API.

The current signature for copy is: = (node, to, transformFunction, expandAncestors = true)

1. I searched SB3 code and it looks like there are only two places where there are calls to tree.copy, both of which are in the gardener.js. this.tree.copy(options.id, options.parentId, copyTransform) and this.tree.copy(options.id, null, copyTransform). So it doesn't look like there's a copy call where the expandAncestors is set to false. Not a big deal, just wanted to mention it.

2. I was planning on adding this new idx argument after the transformer, thinking it would be the least invasive. But after searching SB3 and only finding two .copy calls, I'm not as worried. So the new signature for copy would be:

Tree.prototype.copy = function (node, to, transformer, idx, expandAncestors = true) {

What do you think? Does that seem ideal or would you prefer something else?

[mattsgarlata]

This is very similar to the move method, so we should probably try to get their signatures to be as closely matched as possible. The existing signatures are

Tree.prototype.move = function (node, to, idx, expandAncestors = true) {
Tree.prototype.copy = function (node, to, transformer, expandAncestors = true) {

Based on that, I think it would be good to put idx before transformer so it would be

Tree.prototype.copy = function (node, to, idx, transformer, expandAncestors = true) {

It looks like there's only one spot in the entire codebase of Impact that has expandAncestors=false

[nathanbowser]

Yea I totally agree w/ that, just wanted to see if it would be a problem changing it in SB3.

I'll go w/ that approach.

[mattsgarlata]

No problem changing Impact. We'll just need to be careful to keep Scoreboard on older versions of the tree and only bring this change forward into the summer 2020 commercial release of Impact.