avahe-kellenberger / nimdow

A window manager written in Nim (In Development)
GNU General Public License v2.0
317 stars 19 forks source link

chore: Fix some compiler warnings generated by newer versions of nim #214

Closed marcusramberg closed 7 months ago

avahe-kellenberger commented 1 year ago

I'm seeing an error trying to run nimdow with these changes, here's some info:

I've updated a few things so I can run it locally in Xephyr in debug mode, and get more helpful logs. Here's a patch of those changes:

diff --git a/build.sh b/build.sh
index a3b38a4..4f889b1 100755
--- a/build.sh
+++ b/build.sh
@@ -11,5 +11,10 @@ if [ -n "$(git diff-index --quiet HEAD)" ]; then
 fi

 echo "--- Building with latest commit: $commit ---"
-LATEST_COMMIT="$commit" nim c --multimethods:on -o:bin/nimdow -d:release --opt:speed src/nimdow.nim
+
+if [ "$DEBUG" = "true" ]; then
+  LATEST_COMMIT="$commit" nim c --multimethods:on -o:bin/nimdow -d:debug --linedir:on --debuginfo src/nimdow.nim
+else
+  LATEST_COMMIT="$commit" nim c --multimethods:on -o:bin/nimdow -d:release --opt:speed src/nimdow.nim
+fi

diff --git a/nimdow.nimble b/nimdow.nimble
index eb91649..dc5712f 100644
--- a/nimdow.nimble
+++ b/nimdow.nimble
@@ -16,7 +16,7 @@ requires "https://github.com/avahe-kellenberger/safeset"

 # Tasks
 task debug, "Create a debug build":
-  exec "nim --multimethods:on -o:bin/nimdow --linedir:on --debuginfo c src/nimdow.nim"
+  exec "DEBUG=true ./build.sh"

 task release, "Build for release":
   exec "./build.sh"
diff --git a/src/nimdowpkg/event/xeventmanager.nim b/src/nimdowpkg/event/xeventmanager.nim
index 9a758a3..5a4a8b2 100644
--- a/src/nimdowpkg/event/xeventmanager.nim
+++ b/src/nimdowpkg/event/xeventmanager.nim
@@ -42,6 +42,8 @@ proc removeListener*(this: XEventManager, listener: XEventListener, types: varar
       this.listenerMap[theType].excl(listener)

 when defined(debug):
+  import x11/x
+  import ../logger
   proc logEventTypeName(e: XEvent) =
    case e.theType:
       of KeyPress:

You can build nimdow in debug mode with nimble debug after that patch is applied ^

It should crash immediately. The error I see:

└[avahe@throne $marcus/fix_warnings]: DISPLAY=:1 ./bin/nimdow
/home/avahe/programming/nim/nimdow/src/nimdow.nim(45) nimdow
/home/avahe/programming/nim/nimdow/src/nimdowpkg/ipc/cli.nim(77) handleCommandLineParams
/home/avahe/.choosenim/toolchains/nim-1.6.12/lib/system/fatal.nim(54) sysFatal
Error: unhandled exception: index out of bounds, the container is empty [IndexDefect]

Sorry it's taken me so long to get back on this PR, I was a bit sidetracked.

avahe-kellenberger commented 1 year ago

Sorry, I don't mean to ignore your work. The more I discovered about the updates to error handling, the less confident I became in these changes preventing crashes in some cases.

I think a lot of these try/except blocks could be reworked so that they are no longer needed in the first place. I should go through the code base sometime and simplify those cases (I was pretty new to Nim when I started this project, lol)

marcusramberg commented 7 months ago

No worries. :)