Pass table data to template #137
Conversation
|
I've used the request review feature so its pinned to my todo list and doesnt get lost. |
There was a problem hiding this comment.
I've noticed that the current multiSelectQuestionTemplate contains %s formatting directives. Then when we update the table, the survey.MultiSelectQuestionTemplate is set via fmt.Sprintf(multiSelectQuestionTemplate). This means that the multiSelectQuestionTemplate isn't actually a valid template.
Is it possible to pass extra data where the template is rendered? Then rather than using %s you can use .border and .header and we won't need to keep resetting the template.
Yes, I suppose this would technically make more sense as a function than a global variable.
Unfortunately the template data is set internally, here and its return value is used internally where the table is rendered. |
I find it a little bizarre that it is allowed to set the template but there is no easy/obvious way to set additional template data. I have managed to pass some extra data in via the default selection field though main...markylaing:microcloud:table-template |
Of course this won't work if we actually want to set any defaults. Maybe we can submit an issue upstream for this. |
Rather than relying on a method on the table to pass the list of all entries to the template funcs, just pass it in from the template. Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
Actually, by making the map a I ended up using this approach, but with a struct to clearly represent the 3 fields that are passed into the template. So this fully eliminates any need to reassign the template, and we don't need a global table anymore! Yay! I'll open an issue asking for a cleaner way to pass in data to the template though. |
masnax
changed the title
|
I think perhaps the second and third commits could be combined into one? Only because the border and header values are passed into the template one way, then it is immediately changed in the following commit. Apart from that, I think it's good to go. |
Default is an interface{} so we can abuse this to pass the border and
header strings into the template without having to regenerate it.
Since we are co-opting the `Default` field to pass in template data,
this commit adds a new template function which effectively runs the
the functionality defined here:
https://github.com/go-survey/survey/blob/fa37277e6394c29db7bcc94062cb30cd7785a126/multiselect.go#L235-L257
but on the `DefaultRows` field of the templateData.
Signed-off-by: Max Asnaashari <max.asnaashari@canonical.com>
|
@markylaing As you said you were happy with this, I'll dismiss your earlier review and merge. |
masnax
dismissed
markylaing’s stale review
Concerns were addressed, an issue will be created in the upstream project
|
Ah, still can't merge without a review :) |
Fixes an issue where artifacts would be shown in the table, and in rare occasions the whole table would be drawn incorrectly.
The issue was that the
go-surveypackage we use for the selectable table does not re-compile the table template, instead pulling them from a map keyed by the string value of the template.As @markylaing found, there is an
interface{}type onMultiSelectthat we can abuse to pass in our own data into the template. What this means is we no longer need a method to keep track of the data being passed into the template and template functions, so we can assign the template as soon as possible.In order to still preserve the ability to pass in pre-checked values into the table, I've added a field on to the new
templateDatastruct, which alongside theapply_defaultstemplate function, allows setting pre-checked rows like before.