Hi, Tim. Glad to see you there!
1&2&3 First of all I'm not completely satisfacted by this solution myself, but was somewhat convinvced that there is no better way to handle this.
The core of the problem is deleting unused blanks. We can't allow user just decrease the number of blanks, as he/she can accidentally delete useful data. Right now repeat_elements just don't allow to do it.
"The form should just be responsible for determining how many blanks this question acutally uses - that is the code that was there before " - the problem is that code before determined the number of blanks only in validate and save_options functions, that is only when start editing and after submit button is pressed. When the user pressed 'Add 3 blanks' button the number of blanks wasn't handled by form - it was handled by repeat_element instead:
$repeats += $addfieldsno;
(you can see it in patch, deleted). Form before calling repeat_elements use $this->question to determine the number of subquestions - and it isn't updated when user reloads form pressing 'Apply'.
"That is, the specific form should pass in a parameter like $repeatsatstart, which is the minium number of blanks that the thing edited need. Then inside repeat_elements it should do $actualnumrepeats = max($repeatsatstart, $userpref); " - not so easy, as the user can't decrease the number of blanks on the current form right now (because repeat_elements can't get the number of filled blanks from the caller on reloading page) - so with you solution he can't decrease the number of blanks for this questiontype at all once he raised it! That leads me to the conclusion, that userpref must be based on the number of actual filled blanks, so there will be some way to decrease the number of blanks.
I can thought of only one other way to implement it: the caller of repeat_elements must supply a function, which returns the index of last filled blank. As some forms use repeat_elements several times, we either must use a callback function, or pass some of the repeat_elements arguments to this function so it can determine, about what set of blanks we are asking (it probably must be a prefix, see 5 below).
Also, if I understand you code rightly, question editing form sometimes used without an actual editing question ($this->question->formoptions->repeatelements=false). Setting numofrepeats to max($repeatsatstart, $userpref) in that case will create strange results. The interface to ajust the number of blanks probably should be hidden in that case too. Can we really rely on assumption that blanks can be added in any case without harm to anything?
So, what you would like to see implemented there? Additional form function and user preferences in formslib or leave userprefs to the caller? Or can you see a better way to implement this?
"For example, if you are creating a lot of matching questions with 5 or 6 pairs of answers" - we can easily handle this by adding QUESTION_NUMANS_ADD to user preferences when creating new question. Fixed. I think that QUESTION_NUMANS_ADD should be lowered to 2 in it's new function.
4. I don't like it too, but somewhat short of options. To place text and button on one line we must create a group. The way PEAR handled compare rule is strange: you must supply an array of elements to it. Since adding group rule you must supply an element name as an index, you can't use compare rule in such way - array can't be an index. There are two other options:
a) validate it on server in repeat_elements (without rules at all) - this will costs additional page reloads; using rule would also be the only way to display an error message to describe what is happening, if Jamie would fix
MDL-17111 since definition function can't show any errors. However this would be only way with additional function.
b) use a dropdown - but at least on widely used Internet Explorer the way it hanldes trys to enter the number is quite strange. My opinion is that having to type much (or browse large dropdown) is worse than get one more line (on already big page, where no one will notice it).
I beg you to at least consider these backsides of grouping (additional page reloads and no error messages or inconviniences to get required number) before insisting of grouping. We can group without better options if we will use caller-supplied function to determine the number of blanks thought.
I can easily make an option to not use separate fieldset thought. Is one or two lines really matters? It is fieldsets that actually takes a lot of place (especially fieldsets for blanks). Pity we can't use some sort of horizontal line to separate blanks instead - this will realy makes a difference for question editing.
5. Actually yes. repeat_elements can recieve a prefix for additional elements names ($repeathiddenname, $repeatname, $groupname, $headername, $addfieldsname) instead of getting actual names.
So, I said my reasons. Whats now you decision on:
A) Should we use an additional function, supplied by form, to determine the number of filled blanks, or just leave preferences to the caller, or use some better way I just can't see?
B) How should we treat case with $this->question->formoptions->repeatelements=false? Elements to handle the number of blanks probably should be hidden at all in that case, and userprefs ignored as well. Probably there must be an option to do so in repeat_elements.
C) What is a lesser evil: page reloading (group with text field), inconvinience entering the number of blanks (group with dropdown) or placing 'Aplly' button on separate line? Should there be an option to not use separate fieldset for controls?
D) Should we pass a prefix instead of element's names? We can win 4 arguments (but 2 can be lost for new options mentioned above).
I'll make a new patch according to you decision.