pinnacle-comp/pinnacle

Codebase cleanup and refactor

Ottatop opened this issue · 0 comments

After like 10 months on this project the baggage of my lack of planning is really starting to show up in the codebase.

After #201 is merged I'd like to take some time to evaluate and fix some problems:

  • Split off Backend from the rest of the State struct
    • Because backends are on the same level as the rest of the struct you run into some annoying borrow errors. Both Niri and cosmic-comp split off the backend and stuff the rest of the state into a nested struct.
  • Evaluate the use of user data to store state for windows, tags, and outputs
    • I've already run into some nasty RefCell errors and deadlocks arising from the use of WithState, which uses RefCells stored in user data. I'd like to see if there's a better solution to store this state, preferably without just centralizing it all in the central State.
  • Clean up and modularize the test code
    • #201 is already showing heavy signs of code duplication between Lua and Rust API testing. Should be fairly trivial to unify them.
  • Find a better way to test Lua code
    • Tests for the Lua API are kind of scuffed because they pipe code as a string into the lua executable. Unfortunately lua doesn't exit with an error if there's a syntax error, which has bitten me a few times when writing tests.
  • Evaluate having the default config be always Lua, the hardcoding of luarocks as a build dependency, and build.rs doing the library copying
    • Some of you don't use the Lua API, so having the default config be Lua along with build.rs requiring Luarocks is not ideal. Additionally build.rs does library copying mainly as a developer convenience, but I believe this should be done in something like a Makefile.
  • See if ya can't make the signal code more readable
    • Code for signals was written at like 3 AM in the morning lol. As a result it consists of an overreliance on macros and nested functions with closures to reduce code duplication, but there has to be a better way, right? It's utterly unreadable and I've already had to document what some parameters do to remind myself.
  • Document protobuf nullability (#140) and make this reflect in the type system
    • APIs have a problem with every field in property structs being an option or marked nullable because I haven't done this.
  • Come up with a style guideline for Lua code
    • My goodness is the Lua codebase a mess. There are class definitions threaded between function and method definitions, like 10 different local tables to convert between user-facing strings and protobuf codes, and the docs are a complete mess right now. I need some form of style guidelines to clean this stuff up.
  • Test/rigorize protobuf <-> API type conversions
    • Currently both Lua and Rust APIs operate on the assumption that I won't magically change protobuf definitions, which I absolutely will in the future. Perhaps some tests for converting to and from protobuf would be in order. And for Rust it would be good to impl TryFrom<protobuf_type> for ApiType and vice versa.
  • Unify setting keyboard focus, differentiating between layers and windows
    • It's kind of all over the place

Also more of a reminder for me:

  • There's a resize_output in the udev file that is getting called outside the file, do something about that
  • Do something about the repetition of build_grpc_request_params and all the stuff that's at the top of all the Lua files
  • The Lua string -> protobuf type name conversion is kinda icky