Function definition mutator UI

[AnmAtAnm]

WIP: Compiles, tests, and runs.

Changes:

• Fix bad code on mutator parsing.
• ProcedureManager.mutateProcedure(..), including managing Inputs and making variables.
• Major refactor of NameManagers to preserve user inputed case. Included in this is a real map backing, sorting in NameManager, and index based methods have been shuffled into BasicVariableFieldView.VariableAdapter.

Needs:

• Workspace stats creating variables when a function definition is attached.
• Validate variable fields do the right thing with new NameManager (test pass, and so does a cursory interaction).
• Alert user on variable name conflict.
• Arg name field as drop downs (whoops)

---

This change is 

[AnmAtAnm]

Review status: 0 of 43 files reviewed at latest revision, 7 unresolved discussions.

---

blocklylib-core/src/main/java/com/google/blockly/android/control/ProcedureManager.java, line 240 at r1 (raw file):

Previously, RoboErikG wrote…

Docs for parameters

Done, with a refactor that includes optional / @Nullable arguments (otherwise using existing values).

---

blocklylib-core/src/main/java/com/google/blockly/android/control/ProcedureManager.java, line 260 at r1 (raw file):

Previously, RoboErikG wrote…

Should you check if it's valid before you add it to the manager?

Removed the above name manager addition here, because it was also done within groupAndFireEvents(..), where it belonged, after all vars had already been validated.

---

blocklylib-core/src/main/java/com/google/blockly/android/control/ProcedureManager.java, line 319 at r1 (raw file):

Previously, RoboErikG wrote…

It's bumping it away from another connection, not away from its current position. This means it always ends up the same distance from the other connection. It should be fine to make that optional though and bump from the current position.

Acknowledged. Clarified TODO.

---

blocklylib-core/src/main/java/com/google/blockly/android/ui/fieldview/BasicFieldVariableView.java, line 175 at r1 (raw file):

Previously, RoboErikG wrote…

remove?

Removed, and other in test class.

---

blocklylib-core/src/main/java/com/google/blockly/android/ui/fieldview/BasicFieldVariableView.java, line 249 at r1 (raw file):

Previously, RoboErikG wrote…

You've been converting a lot of direct access loops to iterator loops. These are slightly slower and cause a bit more garbage collection. It's not significant, but I'd prefer you not convert them unless there's a reason to.

Understood about that trade offs. I also realize the TreeSet replacing the SimpleArrayMap has similar trade-offs.

In this case, SortedSet does not have random access, so iterator is the only way to go. The alternative would be to make this return an array backed data structure. If there was a sorted variant of the ArraySet, I'd probably use that.

---

blocklylib-core/src/main/java/com/google/blockly/android/ui/fieldview/BasicFieldVariableView.java, line 303 at r1 (raw file):

Previously, RoboErikG wrote…

ditto

Same SortedSet mVar as above.

---

blocklylib-core/src/main/java/com/google/blockly/model/Workspace.java, line 238 at r1 (raw file):

Previously, RoboErikG wrote…

Here too.

Again, a set. No random access.

---

Comments from Reviewable

[AnmAtAnm]

Awaiting LG

---

Review status: 0 of 43 files reviewed at latest revision, 7 unresolved discussions.

---

Comments from Reviewable

[RoboErikG]

:

---

Review status: 0 of 43 files reviewed at latest revision, 8 unresolved discussions.

---

blocklylib-core/src/main/java/com/google/blockly/android/control/ProcedureManager.java, line 255 at r2 (raw file):

     *                                  name, or if argument name is invalid.
     */
    public void mutateProcedure(final String originalProcedureName,

@NonNull for the originalProcedureName?

---

Comments from Reviewable