[WIP] Refactor

[BlueDrink9]

TODO

• Split script into separate files (Fixes Split script into logically seperated multiple files #34)
• Convert labels to functions
• Refactor variable names
• Move any vim* functions and variables into a vim class. (If possible, add a file containing backwards-compatible stubs).
• Refactor GUI code to add all gui elements using functions.
• (If possible) refactor pages of hotkeys to use macros.
• General refactor of functions, variables, code position.

[BlueDrink9]

@rcmdnk can you tell me some of the functions/variables you call from other scripts? What you might consider most similar to a public API? I'll create wrappers so that they don't lose backwards-compatibility.

[rcmdnk]

Values of Options in README should be changeable from the parent script.
But if we want to classes for this script, CUI setting should be changed, too.

Anyway, I think we should start to make classes before splitting files.
Otherwise, we can not decide the policy of splitting.

[BlueDrink9]

The reverse also applies: to know the classes, we have to spit the script anyway

[BlueDrink9]

But sure, any settings will have an api left for them, same as current.

[rcmdnk]

I'm starting to implement classes
and replace labels to functions to reduce global variables.

https://github.com/rcmdnk/vim_ahk/blob/class/vim.ahk

I think this kind of work makes the way to split files clearly, too,
although the above branch is still under development and not so beautiful.

[BlueDrink9]

Ok, if I discard my changes and pull yours, then make edits, are you happy to cherry-pick my changes too? Or do you want to make a separate PR?

I haven't done a collaborative PR before so I don't know how easy git makes it to add commits from two authors. Can you push directly to this PR branch??

[rcmdnk]

If you agree with class branch structure, then it is easier to start from the branch.

[BlueDrink9]

Sure. I'll reset this PR to your branch, and push changes. What is your timezone and working hours? I'm GMT+12, working 0900-2000. So long as we push frequently, I'll pull each morning and we shouldn't end up with conflicts.

[BlueDrink9]

It would make the process much easier if you check out and add this pull request as a remote. That way we can both collaborate on it. You have push access to it.

[BlueDrink9]

To avoid having to name things the redundant vim[name], the includes could be wrapped in a class in the main script, like:

class Vim{
    #Include %A_LineFile%\..\lib\vim_about.ahk
    #Include %A_LineFile%\..\lib\vim_conf.ahk
    #Include %A_LineFile%\..\lib\vim_debug.ahk
    #Include %A_LineFile%\..\lib\vim_icon_mng.ahk
    #Include %A_LineFile%\..\lib\vim_ini.ahk
    #Include %A_LineFile%\..\lib\vim_menu.ahk
    #Include %A_LineFile%\..\lib\vim_state.ahk
}

That way, instead of being accessed as VimConf.Verbose they can be accessed like Vim.Conf.Verbose, which I think is clearer and tidier. It keeps things nicely separated.

Also, for backwards compatibility, the main script can include an (optional?) api, like

VimReadIni(){
    Vim.Ini.ReadIni()
}

For variables it gets a bit trickier, but I think you could do something like:

VimVerbose := &Vim.Conf.Verbose

Dereferencing may be an issue though, I'll check it. EDIT: Nope, doesn't work. Variable references might have to be updated by hand.

[rcmdnk]

I'm at GMT+9, but I almost can't touch the script during the working hour.
It could be only in the night or holidays, or sometime when I can get a time...

[rcmdnk]

There are some limitations I have already found.
To minimize the things, I use class variables as global variables, which is not so cool.

Another thing that I could not implement is to implement the class method for SetTimer.
For now, I just make global functions and use them.

Control variables used for the settings also must be global or static, and I just let them global, but there is no good way to specify in VimSetting.Menu function.

[BlueDrink9]

Ok. If you push to this pull request branch, I will help work on it tomorrow. Otherwise I can only let you work on it yourself.

Can you please explain: what do you mean by "use class variables as global variables"?

Control variables used for the settings also must be global or static, and I just let them global, but there is no good way to specify in VimSetting.Menu function.

No good way to specify what? To set variables from outside the script, do you mean?

[BlueDrink9]

Anyway, the changes made so far are a big improvement

[BlueDrink9]

Another thing that I could not implement is to implement the class method for SetTimer.

Are the timers only used to detect when the window changes? I have a function that registers for the Windows system event to notify it when the active window changes, so it's actually more efficient (and instant). Would it be helpful to replace the timers with that?

[rcmdnk]

Can you please explain: what do you mean by "use class variables as global variables"?

For example, VimRestoreIME must be global to use as v-label in settings.
But it is not possible to do something like:

for k, v in VimConf.Conf{
  global %k%
}

then, it must be declared explicitly.
There are too many and will be changed, therefore I just use global and add local to each local variable in the function.

[rcmdnk]

Do these dummies still work now that they are functions, not labels?
Functions work as same as labels in the recent AHK.
You can see a popup on the text in the setting window (such as Verbose level)

I just wanted to replace all labels to functions, as it is more manageable.

[rcmdnk]

Are the timers only used to detect when the window changes?

It is for tooltips, to delay and remove after some time.

vim_ahk/lib/vim_setting.ahk

Line 64 in 58a3696

MouseMove(){

In MouseMove, I tried to use VimDisplayToolTip in the class (which is currently commented out), but it does not work well.
It partly works, but it seems every f := ObjBindMethod(VimSetting, "VimDisplayToolTip") set different object, i.e. VimDisplayToolTip works and make a popup, but it continues to make popup every 1000ms because f := ObjBindMethod(VimSetting, "VimDisplayToolTip") gets different object in VimDisplayToolTip and set off for the different object. (maybe)

[BlueDrink9]

I see. That would make wrapping the includes in a class difficult.

[BlueDrink9]

Ok. I decided to wrap the callback function I was talking about and release it anyway. https://github.com/BlueDrink9/ahk-detect_window_change