nog icon indicating copy to clipboard operation
nog copied to clipboard

refactor update_config

Open TimUntersberger opened this issue 4 years ago • 2 comments

the update_config function is already very unmaintanable and I want to refactor this as soon as possible.

TimUntersberger avatar Aug 15 '20 10:08 TimUntersberger

I thought about this a little bit while I was working on the appbar/titlebar and thought an approach would be to separate what update_config does into conditions, actions and "post-actions"

for example, there's this piece that needs to happen in several scenarios

   for grid in GRIDS.lock().unwrap().iter_mut() {
       grid.display = get_display_by_hmonitor(grid.display.hmonitor);
    }

These are the conditions that would cause it:

!config.display_app_bar && new_config.display_app_bar 
config.display_app_bar && !new_config.display_app_bar 
!config.remove_task_bar && new_config.remove_task_bar 
config.remove_task_bar && !new_config.remove_task_bar 

And each condition has additional distinct actions that occur when they're met.

If there was a struct that was like

// rusty pseudo-code
enum PostAction { ... }

struct Example {
    condition: Fn(config, config) -> bool,
    action: Fn(),
    post_action: PostAction
}

Then there could be code that sets up a vec of these structs and then each time the config needs to update it can iterate it like this

let mut post_actions = Vec::<PostAction>::new();
for configChecker in configCheckers {
    if configChecker.condition(old_config, new_config) {
        configChecker.action();
        post_actions.push(configChecker.post_action);
    }
}

post_actions.dedup();
for post_action in post_actions {
    match post_action {
           PostAction::blah => {
                  // this thing here
                    for grid in GRIDS.lock().unwrap().iter_mut() {
                         grid.display = get_display_by_hmonitor(grid.display.hmonitor);
                    }
           },
           // more postactions here
    }
}

It was just something I thought of while looking at that but didn't want to rewrite that bit without a discussion

ramirezmike avatar Aug 22 '20 00:08 ramirezmike

I really like your idea of having "actions". Maybe we could extract the whole logic into actions, instead of only post actions?

enum Action {
    TaskbarShow,
    TaskbarHide,
    ...
}

// don't know how to call this
type Condition = ...;
let temp: Vec<(Condition, Vec<Action>)> = vec![
  .....
]

let mut actions: Vec<Action> = Vec::new();

// We could then go over all of this `temp` and then if it matches we add the actions. 
// Afterwards we "dedup" all of the actions.
// Go over each action and do something

Maybe we would need to save some form of index for the actions, because some of them have to be called in a specific order.

It would be cool if we could somehow calculate the diff of the old config and the new one. This would enable the Condition to be an array of field names instead of manual checks.

To get the diff we could write a macro that creates a function based on the Config struct, which calculates the diff.

let temp = vec![(
    vec!["remove_task_bar"], 
    |old_config, new_config| { // old_config and new_config can be used to conditionally apply an action
        vec![
            Action::TaskbarShow,
            Action::BarClose,
            Action::GridDisplaysUpdate,
        ]
    }
)];

let diff = old_config.diff(new_config);

for x in temp {
    ...
}

TimUntersberger avatar Aug 22 '20 11:08 TimUntersberger