Make all globals local

[bjornbm]

Ref #8 (comment)

Not expecting you to pull this urgently since you mentioned you need to review your init files, but here is a draft for when you are ready (may well need to be updated for merge conflicts).

Note that the first commit 8280237 isn't strictly necessary, but doing loading in one place makes the second commit cleaner. Maybe considering this one regardless of the second two?

[philanc]

I would like to apply the first commit (Separate locating and loading of init file. but I'm afraid I don't know how to merge only this commit without the rest of the PR.

Google told me how to do it with Github Desktop, but I don't have it installed (and don't want to install it).

Can I do it from the github web interface?

Of course I could pickup the diff and apply it manually but I'd rather merge your commit to keep attribution.

[bjornbm]

I think you can take just that commit with git cherry-pick 82802371400868d148940e22c38cdb974b269e5c. I imagine you need to have a copy of the branch locally so git finds the commit.

If it doesn't work, I'll make a new branch with only this commit next week when I am back at my computer.

[philanc]

I think you can take just that commit with git cherry-pick 8280237. I imagine you need to have a copy of the branch locally so git finds the commit.

Thanks. Will look into git cherry-pick and how to import a branch from another repo -- at the moment, my git experience is very limited :)

[philanc]

Your comments about unused variables and turning globals into locals made me think about the code structure and the extension API (to write code in ple_init.lua).

I think that the editor object is too large and gives access to all internals which is also dangerous.

Also ple_init functions should not have direct access to buffers. The buffer parameter 'b' passed to all 'editor.actions' functions is often unused and is (or could be) always the current buffer.

So I have split the 'editor' table in two:

• 'editor' contains all the internal structures and functions (including buffers)
• 'eapi' contains the structures and functions that are visible from ple_init code. It includes utility functions (e.g. display a message, read a string with a prompt) and all the "actions" that can be bound to keys.

All the editor "actions" (that are bound to keys) are stored in 'eapi'. They all apply to the current buffer, so the 'b' parameter (used or not) is removed for all of them.

[bjornbm]

I'll go through the diff of your commits in more details, but my understanding is that you basically addressed the main purpose this pull request by a refactor?

Regarding the exposed api, if you think it is prettier, or perhaps for the potential of backwards compatibility, you could change naming (in two steps):

1. editor -> internal (or einternal)
2. eapi -> editor

Thus keeping the editor for the exposed api, which I think looks friendlier than the somewhat cryptic eapi 😄. Maybe this can also allow compatibility-by-luck for init files that weren't using the now-internal functions.

[philanc]

(...) my understanding is that you basically addressed the main purpose this pull request by a refactor?

Yes. It was also an opportunity to reduce the amount of editor internals exposed in the extension API. I have wanted to do that for a long time because the internals (buffer management, undo machinery) are not so simple, and could be involuntarily broken by some extension code.

Regarding renaming eapi and editor: I like this idea. Will rename eapi into editor. Regarding the former editor (internals), since it is not exported anymore, I may just just remove the editor table and turn its components into local variables.

Edit: Hum... thinking about it, the refactoring will be much easier if editor is just renamed into something else as you suggested.

[bjornbm]

Great. Before I close this pull request, what do you think about 933b2b3?

[bjornbm]

I think this one could basically be closed, but was wondering if you considered 933b2b3.

A benefit of making editor local would be that there is no risk of:

1. accidentally using it in, for example, buffer.lua (I'm sure you wouldn't but maybe a well-meaning patch might…), or
2. something silly like ple_init setting editor = nil breaking the editor (which now will cause the below but with 933b2b3 does not crash).

/Users/bjorn/repos/ple/ple.lua:220: attempt to index a nil value (global 'editor')                                              
stack traceback:
	/Users/bjorn/repos/ple/ple.lua:220: in field 'statusline'
	/Users/bjorn/repos/ple/ple.lua:348: in upvalue 'adjcursor'
	/Users/bjorn/repos/ple/ple.lua:391: in upvalue 'redisplay'
	/Users/bjorn/repos/ple/ple.lua:416: in field 'fullredisplay'
	/Users/bjorn/repos/ple/ple.lua:797: in field 'newbuffer'
	/Users/bjorn/repos/ple/ple.lua:1085: in function </Users/bjorn/repos/ple/ple.lua:1080>
	[C]: in function 'xpcall'
	/Users/bjorn/repos/ple/ple.lua:1133: in local 'main'
	/Users/bjorn/repos/ple/ple.lua:1147: in main chunk
	[C]: in function 'dofile'
	/Users/bjorn/bin/ple:5: in main chunk
	[C]: in 

[bjornbm]

Superseded by #14.