From a709a929d90bf0ba6f6abaf58ef1c5e38262efeb Mon Sep 17 00:00:00 2001 From: Trevor Slocum Date: Mon, 28 Sep 2020 22:34:38 -0700 Subject: [PATCH] Fix FormItem.SetAttributes race condition Resolves #35. --- checkbox.go | 34 ++++---- dropdown.go | 34 ++++---- form.go | 234 ++++++++++++++------------------------------------ go.mod | 4 +- go.sum | 8 +- inputfield.go | 34 ++++---- 6 files changed, 120 insertions(+), 228 deletions(-) diff --git a/checkbox.go b/checkbox.go index 282d8a9..8b96d83 100644 --- a/checkbox.go +++ b/checkbox.go @@ -135,24 +135,6 @@ func (c *CheckBox) GetMessage() string { return c.message } -// SetAttributes sets the given attributes on the check box. -func (c *CheckBox) SetAttributes(attributes ...FormItemAttribute) { - allAttributes := newFormItemAttributes() - for _, attribute := range attributes { - attribute.apply(allAttributes) - } - - allAttributes.setLabelWidth(&c.labelWidth) - allAttributes.setBackgroundColor(&c.backgroundColor) - allAttributes.setLabelColor(&c.labelColor) - allAttributes.setLabelColorFocused(&c.labelColorFocused) - allAttributes.setFieldTextColor(&c.fieldTextColor) - allAttributes.setFieldTextColorFocused(&c.fieldTextColorFocused) - allAttributes.setFieldBackgroundColor(&c.fieldBackgroundColor) - allAttributes.setFieldBackgroundColorFocused(&c.fieldBackgroundColorFocused) - allAttributes.setFinishedFunc(&c.finished) -} - // SetLabelWidth sets the screen width of the label. A value of 0 will cause the // primitive to use the width of the label string. func (c *CheckBox) SetLabelWidth(width int) *CheckBox { @@ -275,6 +257,22 @@ func (c *CheckBox) SetFinishedFunc(handler func(key tcell.Key)) *CheckBox { return c } +// SetAttributes applies attribute settings to a form item. +func (c *CheckBox) SetAttributes(attrs *FormItemAttributes) { + c.SetLabelWidth(attrs.LabelWidth) + c.SetBackgroundColor(attrs.BackgroundColor) + c.SetLabelColor(attrs.LabelColor) + c.SetLabelColorFocused(attrs.LabelColorFocused) + c.SetFieldTextColor(attrs.FieldTextColor) + c.SetFieldTextColorFocused(attrs.FieldTextColorFocused) + c.SetFieldBackgroundColor(attrs.FieldBackgroundColor) + c.SetFieldBackgroundColorFocused(attrs.FieldBackgroundColorFocused) + + if attrs.FinishedFunc != nil { + c.SetFinishedFunc(attrs.FinishedFunc) + } +} + // Draw draws this primitive onto the screen. func (c *CheckBox) Draw(screen tcell.Screen) { c.Box.Draw(screen) diff --git a/dropdown.go b/dropdown.go index 2ff00e9..af29586 100644 --- a/dropdown.go +++ b/dropdown.go @@ -263,24 +263,6 @@ func (d *DropDown) GetLabel() string { return d.label } -// SetAttributes sets the given attributes on the drop-down. -func (d *DropDown) SetAttributes(attributes ...FormItemAttribute) { - allAttributes := newFormItemAttributes() - for _, attribute := range attributes { - attribute.apply(allAttributes) - } - - allAttributes.setLabelWidth(&d.labelWidth) - allAttributes.setBackgroundColor(&d.backgroundColor) - allAttributes.setLabelColor(&d.labelColor) - allAttributes.setLabelColorFocused(&d.labelColorFocused) - allAttributes.setFieldTextColor(&d.fieldTextColor) - allAttributes.setFieldTextColorFocused(&d.fieldTextColorFocused) - allAttributes.setFieldBackgroundColor(&d.fieldBackgroundColor) - allAttributes.setFieldBackgroundColorFocused(&d.fieldBackgroundColorFocused) - allAttributes.setFinishedFunc(&d.finished) -} - // SetLabelWidth sets the screen width of the label. A value of 0 will cause the // primitive to use the width of the label string. func (d *DropDown) SetLabelWidth(width int) *DropDown { @@ -540,6 +522,22 @@ func (d *DropDown) SetFinishedFunc(handler func(key tcell.Key)) *DropDown { return d } +// SetAttributes applies attribute settings to a form item. +func (d *DropDown) SetAttributes(attrs *FormItemAttributes) { + d.SetLabelWidth(attrs.LabelWidth) + d.SetBackgroundColor(attrs.BackgroundColor) + d.SetLabelColor(attrs.LabelColor) + d.SetLabelColorFocused(attrs.LabelColorFocused) + d.SetFieldTextColor(attrs.FieldTextColor) + d.SetFieldTextColorFocused(attrs.FieldTextColorFocused) + d.SetFieldBackgroundColor(attrs.FieldBackgroundColor) + d.SetFieldBackgroundColorFocused(attrs.FieldBackgroundColorFocused) + + if attrs.FinishedFunc != nil { + d.SetFinishedFunc(attrs.FinishedFunc) + } +} + // Draw draws this primitive onto the screen. func (d *DropDown) Draw(screen tcell.Screen) { d.Box.Draw(screen) diff --git a/form.go b/form.go index 9f2d512..4996bc2 100644 --- a/form.go +++ b/form.go @@ -11,147 +11,21 @@ import ( // horizontal layouts. var DefaultFormFieldWidth = 10 -// FormItemAttribute represents a form attribute. -type FormItemAttribute interface { - apply(attributes *formItemAttributes) -} +// FormItemAttributes is a set of attributes to be applied. +type FormItemAttributes struct { + // The screen width of the label. A value of 0 will cause the primitive to + // use the width of the label string. + LabelWidth int -// formItemAttributes is a set of attribute setters to be applied. -type formItemAttributes struct { - setLabelWidth func(width *int) - setBackgroundColor func(color *tcell.Color) - setLabelColor func(color *tcell.Color) - setLabelColorFocused func(color *tcell.Color) - setFieldTextColor func(color *tcell.Color) - setFieldTextColorFocused func(color *tcell.Color) - setFieldBackgroundColor func(color *tcell.Color) - setFieldBackgroundColorFocused func(color *tcell.Color) - setFinishedFunc func(handler *func(key tcell.Key)) -} + BackgroundColor tcell.Color + LabelColor tcell.Color + LabelColorFocused tcell.Color + FieldTextColor tcell.Color + FieldTextColorFocused tcell.Color + FieldBackgroundColor tcell.Color + FieldBackgroundColorFocused tcell.Color -func newFormItemAttributes() *formItemAttributes { - return &formItemAttributes{ - setLabelWidth: func(_ *int) {}, - setBackgroundColor: func(_ *tcell.Color) {}, - setLabelColor: func(_ *tcell.Color) {}, - setLabelColorFocused: func(_ *tcell.Color) {}, - setFieldTextColor: func(_ *tcell.Color) {}, - setFieldTextColorFocused: func(_ *tcell.Color) {}, - setFieldBackgroundColor: func(_ *tcell.Color) {}, - setFieldBackgroundColorFocused: func(_ *tcell.Color) {}, - setFinishedFunc: func(_ *func(key tcell.Key)) {}, - } -} - -// funcFormItemAttribute holds the attribute setter. -type funcFormItemAttribute struct { - f func(*formItemAttributes) -} - -// apply invokes the attribute setter. -func (fdo *funcFormItemAttribute) apply(do *formItemAttributes) { - fdo.f(do) -} - -// newFuncAttribute creates a new funcFormItemAttribute. -func newFuncAttribute(f func(*formItemAttributes)) *funcFormItemAttribute { - return &funcFormItemAttribute{ - f: f, - } -} - -// WithLabelWidth creates a form item attribute promise with the given value to be set. -// When applied, sets the screen width of the label. A value of 0 will cause the -// primitive to use the width of the label string. -func WithLabelWidth(labelWidth int) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setLabelWidth = func(width *int) { - *width = labelWidth - } - }) -} - -// WithBackgroundColor creates a form item attribute promise with the given value to be set. -// When applied, sets the background color. -func WithBackgroundColor(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setBackgroundColor = func(backgroundColor *tcell.Color) { - *backgroundColor = color - } - }) -} - -// WithLabelColor creates a form item attribute promise with the given value to be set. -// When applied, sets the color of the label. -func WithLabelColor(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setLabelColor = func(labelColor *tcell.Color) { - *labelColor = color - } - }) -} - -// WithLabelColorFocused creates a form item attribute promise with the given value to be set. -// When applied, sets the color of the label when focused. -func WithLabelColorFocused(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setLabelColorFocused = func(labelColorFocused *tcell.Color) { - *labelColorFocused = color - } - }) -} - -// WithFieldTextColor creates a form item attribute promise with the given value to be set. -// When applied, sets the text color of the input area. -func WithFieldTextColor(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setFieldTextColor = func(fieldTextColor *tcell.Color) { - *fieldTextColor = color - } - }) -} - -// WithFieldTextColorFocused creates a form item attribute promise with the given value to be set. -// When applied, sets the text color of the input area when focused. -func WithFieldTextColorFocused(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setFieldTextColorFocused = func(fieldTextColorFocused *tcell.Color) { - *fieldTextColorFocused = color - } - }) -} - -// WithFieldBackgroundColor creates a form item attribute promise with the given value to be set. -// When applied, sets the background color of the input area. -func WithFieldBackgroundColor(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setFieldBackgroundColor = func(fieldBackgroundColor *tcell.Color) { - *fieldBackgroundColor = color - } - }) -} - -// WithFieldBackgroundColorFocused creates a form item attribute promise with the given value to be set. -// When applied, sets the background color of the input area when focused. -func WithFieldBackgroundColorFocused(color tcell.Color) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setFieldBackgroundColorFocused = func(fieldBackgroundColorFocused *tcell.Color) { - *fieldBackgroundColorFocused = color - } - }) -} - -// WithFinishedFunc creates a form item attribute promise with the given value to be set. -// When applied, sets the handler function for when the user finished -// entering data into the item. The handler may receive events for the -// Enter key (we're done), the Escape key (cancel input), the Tab key (move to -// next field), and the Backtab key (move to previous field). -func WithFinishedFunc(handler func(key tcell.Key)) FormItemAttribute { - return newFuncAttribute(func(o *formItemAttributes) { - o.setFinishedFunc = func(finishedFunc *func(key tcell.Key)) { - *finishedFunc = handler - } - }) + FinishedFunc func(key tcell.Key) } // FormItem is the interface all form items must implement to be able to be @@ -162,9 +36,6 @@ type FormItem interface { // GetLabel returns the item's label text. GetLabel() string - // SetAttributes sets the given attributes on the form item. - SetAttributes(attributes ...FormItemAttribute) - // GetFieldWidth returns the width of the form item's field (the area which // is manipulated by the user) in number of screen cells. A value of 0 // indicates the the field width is flexible and may use as much space as @@ -173,6 +44,9 @@ type FormItem interface { // GetFieldHeight returns the height of the form item. GetFieldHeight() int + + // SetAttributes applies attributes to the form item. + SetAttributes(attrs *FormItemAttributes) } // Form allows you to combine multiple one-line form elements into a vertical @@ -697,6 +571,26 @@ func (f *Form) SetCancelFunc(callback func()) *Form { return f } +// GetAttributes returns the current attribute settings of a form. +func (f *Form) GetAttributes() *FormItemAttributes { + f.Lock() + defer f.Unlock() + + return f.getAttributes() +} + +func (f *Form) getAttributes() *FormItemAttributes { + return &FormItemAttributes{ + BackgroundColor: f.backgroundColor, + LabelColor: f.labelColor, + LabelColorFocused: f.labelColorFocused, + FieldTextColor: f.fieldTextColor, + FieldTextColorFocused: f.fieldTextColorFocused, + FieldBackgroundColor: f.fieldBackgroundColor, + FieldBackgroundColorFocused: f.fieldBackgroundColorFocused, + } +} + // Draw draws this primitive onto the screen. func (f *Form) Draw(screen tcell.Screen) { f.Box.Draw(screen) @@ -757,15 +651,9 @@ func (f *Form) Draw(screen tcell.Screen) { itemWidth = rightLimit - x } - item.SetAttributes( - WithLabelWidth(labelWidth), - WithBackgroundColor(f.backgroundColor), - WithLabelColor(f.labelColor), - WithLabelColorFocused(f.labelColorFocused), - WithFieldTextColor(f.fieldTextColor), - WithFieldTextColorFocused(f.fieldTextColorFocused), - WithFieldBackgroundColor(f.fieldBackgroundColor), - WithFieldBackgroundColorFocused(f.fieldBackgroundColorFocused)) + attributes := f.getAttributes() + attributes.LabelWidth = labelWidth + item.SetAttributes(attributes) // Save position. positions[index].x = x @@ -892,21 +780,8 @@ func (f *Form) Draw(screen tcell.Screen) { } } -// Focus is called by the application when the primitive receives focus. -func (f *Form) Focus(delegate func(p Primitive)) { - f.Lock() - if len(f.items)+len(f.buttons) == 0 { - f.hasFocus = true - f.Unlock() - return - } - f.hasFocus = false - - // Hand on the focus to one of our child elements. - if f.focusedElement < 0 || f.focusedElement >= len(f.items)+len(f.buttons) { - f.focusedElement = 0 - } - handler := func(key tcell.Key) { +func (f *Form) formItemInputHandler(delegate func(p Primitive)) func(key tcell.Key) { + return func(key tcell.Key) { f.Lock() switch key { @@ -945,18 +820,41 @@ func (f *Form) Focus(delegate func(p Primitive)) { f.Unlock() } +} + +// Focus is called by the application when the primitive receives focus. +func (f *Form) Focus(delegate func(p Primitive)) { + f.Lock() + if len(f.items)+len(f.buttons) == 0 { + f.hasFocus = true + f.Unlock() + return + } + f.hasFocus = false + + // Hand on the focus to one of our child elements. + if f.focusedElement < 0 || f.focusedElement >= len(f.items)+len(f.buttons) { + f.focusedElement = 0 + } if f.focusedElement < len(f.items) { // We're selecting an item. item := f.items[f.focusedElement] - item.SetAttributes(WithFinishedFunc(handler)) + + attributes := f.getAttributes() + attributes.FinishedFunc = f.formItemInputHandler(delegate) + f.Unlock() + + item.SetAttributes(attributes) delegate(item) } else { // We're selecting a button. button := f.buttons[f.focusedElement-len(f.items)] - button.SetBlurFunc(handler) + button.SetBlurFunc(f.formItemInputHandler(delegate)) + f.Unlock() + delegate(button) } } diff --git a/go.mod b/go.mod index ca02ee0..871f26f 100644 --- a/go.mod +++ b/go.mod @@ -3,10 +3,10 @@ module gitlab.com/tslocum/cview go 1.12 require ( - github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200908121250-0c5e1e1720f1 + github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200926152101-0fb77ddaa5b4 github.com/lucasb-eyer/go-colorful v1.0.3 github.com/mattn/go-runewidth v0.0.9 github.com/rivo/uniseg v0.1.0 gitlab.com/tslocum/cbind v0.1.2 - golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d // indirect + golang.org/x/sys v0.0.0-20200928205150-006507a75852 // indirect ) diff --git a/go.sum b/go.sum index 5cbfef0..ddab79d 100644 --- a/go.sum +++ b/go.sum @@ -1,8 +1,8 @@ github.com/gdamore/encoding v1.0.0 h1:+7OoQ1Bc6eTm5niUzBa0Ctsh6JbMW6Ra+YNuAtDBdko= github.com/gdamore/encoding v1.0.0/go.mod h1:alR0ol34c49FCSBLjhosxzcPHQbf2trDkoo5dl+VrEg= github.com/gdamore/tcell/v2 v2.0.0-dev/go.mod h1:vSVL/GV5mCSlPC6thFP5kfOFdM9MGZcalipmpTxTgQA= -github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200908121250-0c5e1e1720f1 h1:ec/DAe6ms4fBkpSHObVDYU4N/w6Swd929zkN01g8ozY= -github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200908121250-0c5e1e1720f1/go.mod h1:vSVL/GV5mCSlPC6thFP5kfOFdM9MGZcalipmpTxTgQA= +github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200926152101-0fb77ddaa5b4 h1:9WLVV5c2UI2qvgROlgzLgCuK5gi7igcU5LNsPXCSFB8= +github.com/gdamore/tcell/v2 v2.0.0-dev.0.20200926152101-0fb77ddaa5b4/go.mod h1:vSVL/GV5mCSlPC6thFP5kfOFdM9MGZcalipmpTxTgQA= github.com/lucasb-eyer/go-colorful v1.0.3 h1:QIbQXiugsb+q10B+MI+7DI1oQLdmnep86tWFlaaUAac= github.com/lucasb-eyer/go-colorful v1.0.3/go.mod h1:R4dSotOR9KMtayYi1e77YzuveK+i7ruzyGqttikkLy0= github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI= @@ -15,8 +15,8 @@ gitlab.com/tslocum/cbind v0.1.2/go.mod h1:HfB7qAhHSZbn1rFK8M9SvSN5NG6ScAg/3h3iE6 golang.org/x/sys v0.0.0-20190626150813-e07cf5db2756/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/sys v0.0.0-20200826173525-f9321e4c35a6 h1:DvY3Zkh7KabQE/kfzMvYvKirSiguP9Q/veMtkYyf0o8= golang.org/x/sys v0.0.0-20200826173525-f9321e4c35a6/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= -golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d h1:L/IKR6COd7ubZrs2oTnTi73IhgqJ71c9s80WsQnh0Es= -golang.org/x/sys v0.0.0-20200923182605-d9f96fdee20d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= +golang.org/x/sys v0.0.0-20200928205150-006507a75852 h1:sXxgOAXy8JwHhZnPuItAlUtwIlxrlEqi28mKhUR+zZY= +golang.org/x/sys v0.0.0-20200928205150-006507a75852/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.3 h1:cokOdA+Jmi5PJGXLlLllQSgYigAEfHXJAERHVMaCc2k= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= diff --git a/inputfield.go b/inputfield.go index f006ae1..6bdff76 100644 --- a/inputfield.go +++ b/inputfield.go @@ -198,24 +198,6 @@ func (i *InputField) GetLabel() string { return i.label } -// SetAttributes sets the given attributes on the input field. -func (i *InputField) SetAttributes(attributes ...FormItemAttribute) { - allAttributes := newFormItemAttributes() - for _, attribute := range attributes { - attribute.apply(allAttributes) - } - - allAttributes.setLabelWidth(&i.labelWidth) - allAttributes.setBackgroundColor(&i.backgroundColor) - allAttributes.setLabelColor(&i.labelColor) - allAttributes.setLabelColorFocused(&i.labelColorFocused) - allAttributes.setFieldTextColor(&i.fieldTextColor) - allAttributes.setFieldTextColorFocused(&i.fieldTextColorFocused) - allAttributes.setFieldBackgroundColor(&i.fieldBackgroundColor) - allAttributes.setFieldBackgroundColorFocused(&i.fieldBackgroundColorFocused) - allAttributes.setFinishedFunc(&i.finished) -} - // SetLabelWidth sets the screen width of the label. A value of 0 will cause the // primitive to use the width of the label string. func (i *InputField) SetLabelWidth(width int) *InputField { @@ -573,6 +555,22 @@ func (i *InputField) SetFinishedFunc(handler func(key tcell.Key)) *InputField { return i } +// SetAttributes applies attribute settings to a form item. +func (i *InputField) SetAttributes(attrs *FormItemAttributes) { + i.SetLabelWidth(attrs.LabelWidth) + i.SetBackgroundColor(attrs.BackgroundColor) + i.SetLabelColor(attrs.LabelColor) + i.SetLabelColorFocused(attrs.LabelColorFocused) + i.SetFieldTextColor(attrs.FieldTextColor) + i.SetFieldTextColorFocused(attrs.FieldTextColorFocused) + i.SetFieldBackgroundColor(attrs.FieldBackgroundColor) + i.SetFieldBackgroundColorFocused(attrs.FieldBackgroundColorFocused) + + if attrs.FinishedFunc != nil { + i.SetFinishedFunc(attrs.FinishedFunc) + } +} + // Draw draws this primitive onto the screen. func (i *InputField) Draw(screen tcell.Screen) { i.Box.Draw(screen)