refactor: The whole project #6

cilginc · 2025-10-15T18:18:11Z

No description provided.

ThatOtherAndrew · 2025-10-15T20:35:49Z

Checking in with this from time to time and it's really impressive stuff, thank you so much for your work on this!

cilginc · 2025-10-15T22:38:30Z

You can merge now. FYI, I had to apply this workaround across multiple parts of the code, so it’s not fully idiomatic yet.

type App struct {
	app *models.App
}

func New(app *models.App) *App {
	return &App{app: app}
}

Instead of defining methods on the struct like this:

func (a *App) foo() {}

I prefer using standalone functions that take the struct as an argument:

func foo(a *App) {}

This avoids mutating structs from other packages, which is generally discouraged in Go. It keeps your API simpler and avoids unintentional side effects.

Also, check the TODO comments I added for future refactoring.

Currently, shaders are embedded in the binary, but we could also load them from disk or combine both approaches. Embedding works fine for now and keeps things self-contained.

ThatOtherAndrew · 2025-10-16T23:25:22Z

Very curious to test out this copilot review feature.

Don't worry, will be reviewing manually too (hence the slow merge - sorry!)

Copilot

Pull Request Overview

Major refactor to modularize the project, move Wayland/C interop into a dedicated package, externalize shaders, and introduce a proper cmd entrypoint.

Extracted Wayland integration into pkg/wayland with xkbcommon-based key handling.
Split application into internal packages (models, draw, update, spawn, stroke, gesture, execute, opengl) and moved main to cmd/hexecute.
Externalized GLSL shaders to files (with go:embed) and added shader compilation helpers.
Updated module path to GitHub and adjusted project structure.

Reviewed Changes

Copilot reviewed 31 out of 35 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
wayland.go	Removed legacy monolithic Wayland/C interop (replaced by pkg/wayland).
pkg/wayland/wlr-layer-shell-client.h	Formatting/consistency tweaks to autogenerated header.
pkg/wayland/wayland.h	New C header: Wayland/XKB declarations and exported functions.
pkg/wayland/wayland.go	New Go wrapper for Wayland/EGL window lifecycle and input.
pkg/wayland/wayland.c	New C implementation for Wayland glue, XKB, and input tracking.
pkg/wayland/keyboard-shortcuts-inhibit-client.h	Formatting/consistency tweaks to autogenerated header.
pkg/wayland/.clangd	Added editor config for C/C++ completions (non-functional change).
main.go	Removed monolithic main (replaced by cmd/hexecute).
internal/update/update.go	New package for cursor/particle update logic.
internal/stroke/stroke.go	Ported stroke/gesture logic into internal package.
internal/spawn/spawn.go	Particle spawning helpers split out.
internal/shaders/shader.go	Shader compilation helpers (from file and from source).
internal/shaders/paths.go	Shader paths and embedded shader sources.
internal/shaders/*.glsl	GLSL shaders extracted to files.
internal/opengl/opengl.go	OpenGL init and VAO/VBO/program setup.
internal/models/models.go	Centralized data structures (App, Point, Particle, GestureConfig).
internal/gesture/gesture.go	Gesture config load/save helpers.
internal/fornow/fornow.go	Transitional logic: point accumulation and gesture execution.
internal/execute/execute.go	Command execution helper.
internal/draw/draw.go	Rendering logic (background, glow, lines, particles).
internal/config/config.go	Config path helper.
go.mod	Module path updated; go version directive changed.
cmd/hexecute/main.go	New entrypoint wiring packages together and event loop.
README.md	Updated demo asset path.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

internal/opengl/opengl.go

internal/shaders/shader.go

Copilot · 2025-10-16T23:28:38Z

pkg/wayland/wayland.c

+struct xkb_context *xkb_context;
+struct xkb_keymap *xkb_keymap;
+struct xkb_state *xkb_state;


xkb_context, xkb_keymap, and xkb_state are allocated but never released, causing a memory leak. Add a cleanup function (xkb_state_unref, xkb_keymap_unref, xkb_context_unref) and call it from WaylandWindow.Destroy.

Copilot · 2025-10-16T23:28:38Z

cmd/hexecute/main.go

+	if *listGestures {
+		gestures, err := gestures.LoadGestures()
+		if err != nil {
+			log.Fatal("Failed to load gestures:", err)
+		}
+		if len(gestures) == 0 {


[nitpick] The local variable gestures shadows the imported package name 'gestures', which reduces readability and can lead to subtle mistakes. Consider renaming the variable to 'gs' or 'loadedGestures'.

Copilot · 2025-10-16T23:28:39Z

cmd/hexecute/main.go

+					spawn := spawn.New(app)
+					spawn.SpawnExitWisps(float32(x), float32(y))
+				}


[nitpick] The local variable spawn shadows the imported package 'spawn'. Rename the variable (e.g., 'spawner') to avoid confusion; the same pattern appears again when spawning cursor sparkles.

Copilot · 2025-10-16T23:28:39Z

cmd/hexecute/main.go

+			spawn := spawn.New(app)
+			spawn.SpawnCursorSparkles(float32(x), float32(y))


[nitpick] Same shadowing as above; rename the local variable to avoid masking the package import.

Copilot · 2025-10-16T23:28:39Z

internal/opengl/opengl.go

+		logMsg := make([]byte, logLength)
+		gl.GetProgramInfoLog(a.app.Program, logLength, nil, &logMsg[0])
+		log.Fatalf("Failed to link program: %s", logMsg)


[nitpick] Avoid calling log.Fatalf in a library/init function; prefer returning an error so the caller can decide how to handle failures. This pattern repeats for other program links in this file.

Copilot · 2025-10-16T23:28:39Z

cmd/hexecute/main.go

+type App struct {
+	*models.App
+}
+


[nitpick] This wrapper type is declared but not used in the file; consider removing it to keep the entrypoint lean.

Suggested change

type App struct {

*models.App

}

Finally, accurate information from AI! The issue has been resolved in this PR, though it currently doesn’t affect the project.

ThatOtherAndrew · 2025-10-17T00:12:48Z

if i had a penny for every hallucination here then i wouldn't need employment

ThatOtherAndrew · 2025-10-17T14:43:08Z

@cilginc what's with the fornow.go file?

cilginc · 2025-10-17T14:55:36Z

@cilginc what's with the fornow.go file?

Oh, I forgot to refactor that package before committing.

Working on it.

ThatOtherAndrew

Finally got through all this! And still with 1h20m of overnight sleep remaining 😅

Thanks again for the refactor!

.gitignore

ThatOtherAndrew · 2025-10-19T10:08:58Z

Ready to merge, I believe!

What command is it that you've been using to build the project outside of Nix? Is it something similar to the form go build -o /bin/hexecute ./cmd/hexecute?

cilginc · 2025-10-19T10:10:19Z

Ready to merge, I believe!

What command is it that you've been using to build the project outside of Nix? Is it something similar to the form go build -o /bin/hexecute ./cmd/hexecute?

I use go build -o bin ./....

ThatOtherAndrew · 2025-10-20T01:51:40Z

Didn't know that ellipsis was a syntax, thanks! I'll update the README accordingly.

cilginc added 3 commits October 15, 2025 20:14

feat: Use system-wide Escape key for exiting

b309799

chore: updated github project structure

6d3251d

fix: go module

6f4c655

ThatOtherAndrew mentioned this pull request Oct 15, 2025

feat: Use system-wide Escape key for exiting #4

Closed

cilginc force-pushed the refactor-structure branch from db4b31f to 865b158 Compare October 15, 2025 22:10

cilginc marked this pull request as ready for review October 15, 2025 22:28

cilginc mentioned this pull request Oct 16, 2025

!feat: Updated cli parameters #9

Open

ThatOtherAndrew requested a review from Copilot October 16, 2025 23:24

Copilot AI reviewed Oct 16, 2025

View reviewed changes

cilginc added 15 commits October 17, 2025 18:12

refactor: refactored wayland package

d81576c

refactor: added models package

c53985b

refactor: added stroke package

ae49475

refactor: added shaders package

38c9661

feat: better shader implementation

731daa5

refactor: added config package

b2b216c

refactor: added execute package

24a8a91

refactor: added default dpackage

1731bab

refactor: added gesture package

bcdc939

chore: updated gitignore

d758750

refactor: all the project

2c7426f

chore: handled unhandled error

d58a38c

chore: updated deprecated functions

291b496

feat: embedded the shader files

4e8aa39

chore: deleted fornow package

f47aba9

cilginc force-pushed the refactor-structure branch from 061bb59 to f47aba9 Compare October 17, 2025 15:13

ThatOtherAndrew approved these changes Oct 19, 2025

View reviewed changes

.gitignore Show resolved Hide resolved

fix: added result/ to gitignore for nix build

5c2d709

ThatOtherAndrew added 2 commits October 20, 2025 02:53

Update manual build instructions in readme

1886a0f

Correctly ignore Nix build output

e93dd0a

ThatOtherAndrew merged commit 42e5956 into ThatOtherAndrew:main Oct 20, 2025

Vesper-Works mentioned this pull request Dec 6, 2025

Failure to create a wayland window after updating system #17

Closed

		spawn := spawn.New(app)
		spawn.SpawnCursorSparkles(float32(x), float32(y))

Uh oh!

refactor: The whole project #6

refactor: The whole project #6

Uh oh!

Conversation

cilginc commented Oct 15, 2025

Uh oh!

ThatOtherAndrew commented Oct 15, 2025

Uh oh!

cilginc commented Oct 15, 2025

Uh oh!

ThatOtherAndrew commented Oct 16, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 16, 2025

Choose a reason for hiding this comment

Uh oh!

cilginc Oct 17, 2025

Choose a reason for hiding this comment

Uh oh!

ThatOtherAndrew commented Oct 17, 2025

Uh oh!

ThatOtherAndrew commented Oct 17, 2025

Uh oh!

cilginc commented Oct 17, 2025

Uh oh!

ThatOtherAndrew left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ThatOtherAndrew commented Oct 19, 2025

Uh oh!

cilginc commented Oct 19, 2025

Uh oh!

ThatOtherAndrew commented Oct 20, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants