Refactoring of widget flags

[mc-butler]

Important

This issue was migrated from Trac:

Origin	https://midnight-commander.org/ticket/3632
Reporter	andrew_b (@aborodin)

Currently, Widget::options is the mix of options and states and it should be split to:

• Widget::options. These flags are set once, when the widget is constructed, and usually stay constant (if widget is input line, it is input line always, for example);
• Widget::state. This field contains information on the state of the widget. Unlike option flags, state flags often change during the lifetime of a widget as the state of the widget changes.
• WDialog::state should be merged into Widget::state as they are defines dialog state;
• WDialog::flags should be merged into Widget::options as they are defines dialog options.

[mc-butler]

Changed by andrew_b (@aborodin) on Apr 13, 2016 at 13:02 UTC (comment 1)

• Owner set to andrew_b
• Status changed from new to accepted

Branch: 3632_widget_flags
Initial [a64ba3eb2ddb9a0555a4b5ff7ba76b76cbe9c8fd]

[mc-butler]

Changed by mooffie (@mooffie) on Apr 15, 2016 at 4:10 UTC (comment 2)

Just some random notes:

• Have you considered having MSG_STATE_CHANGED instead of MSG_DISABLE/MSG_ENABLE? (Interestingly, this could solve [ebf57a8] too, though I'm not sure we'd want this as the official solution.)

• Was it wise to rename WANT_IDLE to IDLE? The line "widget_get_state(w, WST_IDLE)" reads as "the widget is idle", which is wrong because there's no such thing "idle widget".

• For the same reason (code clarity), perhaps it's better to return the "DLG" to WST_ACTIVE et al, because WST_ACTIVE doesn't mean "active widget"?

OTOH, I now see that there are going to be a bunch of other "DLG" flags (DLG_CENTER, DLG_TRYUP), and I wonder if having to keep this "DLG" is a code smell.

(It's a bit disappointing to lose dlg->state. The code was quite clear then. But perhaps it's just psychological attachment on my part, which is why I write this in parenthesis. Did it bother you too losing it?)

• As for WOP_TOP_SELECT: I was wondering if it'd be better to postpone this till after we fix the MSG_FOCUS mechanism (and, later, think of how to implement scrollbars) and know more fully what our needs are.

• Out of curiosity: what drove you to introduce this change?

[mc-butler]

Changed by andrew_b (@aborodin) on Apr 15, 2016 at 7:15 UTC (comment 2.3)

Replying to mooffie:

Just some random notes:

• Have you considered having MSG_STATE_CHANGED instead of MSG_DISABLE/MSG_ENABLE?

Change state to what one? Widget state is a set is states: at the same time widget is active, enabled, focused, etc.

• Was it wise to rename WANT_IDLE to IDLE? The line "widget_get_state(w, WST_IDLE)" reads as "the widget is idle", which is wrong because there's no such thing "idle widget".

WANT_IDLE looks like WANT_CURSOR and WAND HOTKEY, i.e. option. Current IDLE is state. I agree, this name is not quite good and can be changed to some more appropriate.

• For the same reason (code clarity), perhaps it's better to return the "DLG" to WST_ACTIVE et al, because WST_ACTIVE doesn't mean "active widget"?

Dialog is inherited from widget and therefore dialog is widget. Every visible object should be widget. This change will be used in the future.

OTOH, I now see that there are going to be a bunch of other "DLG" flags (DLG_CENTER, DLG_TRYUP), and I wonder if having to keep this "DLG" is a code smell.

See [1] below.

(It's a bit disappointing to lose dlg->state. The code was quite clear then. But perhaps it's just psychological attachment on my part, which is why I write this in parenthesis. Did it bother you too losing it?)

See above. Dialog is widget. No reason to have independent state definitions for one widget.

• As for WOP_TOP_SELECT: I was wondering if it'd be better to postpone this till after we fix the MSG_FOCUS mechanism

It does not matter what will be first. The implementation of WOP_TOP_SELECT is simple and simplify the code (get rid of special dlg_set_top_widget() function).

(and, later, think of how to implement scrollbars) and know more fully what our needs are.

See [1] below.

• Out of curiosity: what drove you to introduce this change?

[1] This is one step to #2919. I was not probably create this ticket but continue #2919. The following should be written there but I write it here.

In general: current widget subsystem in mc is very simple: there are only two layers: Widget and WDialog. It is obvious those two objects are not enough to create non-trivial complicated objects. Look at the current implementation of screens (#1490). This is a lot of hacks. In order to implement #2156 we should add more and more hacks.

We want flexible and power hierarchy of widgets. In simplest case this is Widget->WGroup->WDialog.

WGroup represents a set of widgets and it visible from outside as solid widget.

If you wand implement scrollbar within current widget subsystem, you have some problems with it positioning, sizing, drawing, communication with other widget. The scrollable widget should be group, i.e. WEditWinow = WEdit + 2 WScrollbars, WFilePanel = WFileList + WScrollbar(s), WScreen = WMenu + WButtonBar + (WFileManader or WEditor or WViewer or etc).

I'm currently slow working on implementation of WGroup, but this branch is very dirty and far from finish. I decided to split this work to several small steps to refacror current widget subsystem before implementation of WGroup.

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 2, 2016 at 17:49 UTC (comment 4)

• Branch state changed from no branch to on review
• Milestone changed from Future Releases to 4.8.18

Branch: 3632_widget_flags
Initial [619480c]

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 10, 2016 at 11:25 UTC (comment 5)

• Branch state changed from on review to approved
• Votes set to andrew_b

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 10, 2016 at 11:27 UTC (comment 6)

• Branch state changed from approved to merged
• Votes changed from andrew_b to committed-master
• Status changed from accepted to testing
• Resolution set to fixed

Merged to master: [61c379b].

git log --pretty=oneline 5251045..61c379b

[mc-butler]

Changed by andrew_b (@aborodin) on Jun 10, 2016 at 11:28 UTC (comment 7)

• Status changed from testing to closed