martanne / vis

A vi-like editor based on Plan 9's structural regular expressions
Other
4.19k stars 260 forks source link

RFC: Largish Cleanup and Refactoring #1191

Closed rnpnr closed 1 month ago

rnpnr commented 1 month ago

I was growing tired of trying to debug things in vis when everything had to jump through many levels of redirection because all of the core structures were opaque. So I went through and cleaned all that up. While I was at it I shuffled some things around so that now when you want to add a new event to lua you don't need to define a stub function for when lua is disabled.

The only functional change that should have happened is that the QUIT event now happens after all windows have been closed and their files freed. Everything else should be the same as before.

Obviously a 450 line reduction in code is nice and on my system the binary size was also reduced by ~10kB. At runtime there is probably a minor performance benefit and lower memory usage but I didn't really measure those.

I'm still not a fan of how all the header files are interdependent and tangled up but fixing that will have to wait until later.

rnpnr commented 1 month ago

(The failing checks are an issue with the tests on ubuntu. GCC or GLIBC changed something and now the fortified headers do not let you use strncpy to copy an exact number of bytes without room for a NUL terminator. I suspect it can be fixed by replacing strncpy with memcpy but I can't really test because its not broken on my system.)

rnpnr commented 1 month ago

compiler should inline most of the get-/ set-methods anyway.

Not really, since they are defined in different translation units. Hence the 10k reduction in binary size.

Not sure though, what I think about moving validity checks from some set-methods to their call sides.

I think most of things I removed were not doing that. The ones that were checking for validity were also checking for validity at their call site so the work was just being doubled for no reason. That is the primary benefit of being explicit about what you are doing rather than calling some function and ignoring the internals of the function (as you should).