Skip to content

Allow running the project #139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2025
Merged

Allow running the project #139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cli/nhcli.cabal
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ library
Neo.Build.Templates.AppMain,
Neo.Core,
Neo.Core.ProjectConfiguration,
Neo.Run,
-- other-modules:
-- other-extensions:
hs-source-dirs: src
Expand Down
29 changes: 27 additions & 2 deletions cli/src/Neo.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Core
import File qualified
import Json qualified
import Neo.Build qualified as Build
import Neo.Run qualified as Run
import Task qualified
import Text qualified

Expand All @@ -23,6 +24,7 @@ data CommonFlags = CommonFlags

data NeoCommand
= Build CommonFlags
| Run CommonFlags
deriving (Show, Eq, Ord)


Expand Down Expand Up @@ -55,12 +57,19 @@ commandsParser = do
let build =
Command.CommandOptions
{ name = "build",
description = "build a file or directory",
description = "build the project",
version = Nothing,
decoder = buildParser
}
let run =
Command.CommandOptions
{ name = "run",
description = "run the project",
version = Nothing,
decoder = runParser
}
Command.commands
(Array.fromLinkedList [build])
(Array.fromLinkedList [build, run])


buildParser :: Command.OptionsParser NeoCommand
Expand All @@ -69,6 +78,12 @@ buildParser = do
pure (Build common)


runParser :: Command.OptionsParser NeoCommand
runParser = do
common <- flagsParser
pure (Run common)


flagsParser :: Command.OptionsParser CommonFlags
flagsParser = do
projectFilePath <-
Expand All @@ -85,6 +100,7 @@ flagsParser = do

data Error
= BuildError Build.Error
| RunError Run.Error
| Other
deriving (Show)

Expand All @@ -99,3 +115,12 @@ handleCommand command =
Ok config ->
Build.handle config
|> Task.mapError (\e -> BuildError e)
Run flags -> do
txt <- File.readText flags.projectFile |> Task.mapError (\_ -> Other)
case Json.decodeText txt of
Err err -> panic err
Ok config -> do
Build.handle config
|> Task.mapError (\e -> BuildError e)
Run.handle config
|> Task.mapError (\e -> RunError e)
Comment on lines +118 to +126
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

CONSIDER A MORE EXPLICIT BUILD-THEN-RUN FLOW!

The current implementation always executes Build.handle before Run.handle which is logically sound, but could benefit from more explicit error handling between these steps.

Consider this more explicit approach:

Run flags -> do
  txt <- File.readText flags.projectFile |> Task.mapError (\_ -> Other)
  case Json.decodeText txt of
    Err err -> panic err
    Ok config -> do
      -- Build first
      buildResult <- 
        Build.handle config
          |> Task.mapError (\e -> BuildError e)
+         |> Task.attempt
+      
+      -- Only run if build was successful
+      case buildResult of
+        Err err -> Task.throw err
+        Ok _ ->
          Run.handle config
            |> Task.mapError (\e -> RunError e)

This approach makes it clear that the run step only proceeds if the build step succeeds.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Run flags -> do
txt <- File.readText flags.projectFile |> Task.mapError (\_ -> Other)
case Json.decodeText txt of
Err err -> panic err
Ok config -> do
Build.handle config
|> Task.mapError (\e -> BuildError e)
Run.handle config
|> Task.mapError (\e -> RunError e)
Run flags -> do
txt <- File.readText flags.projectFile |> Task.mapError (\_ -> Other)
case Json.decodeText txt of
Err err -> panic err
Ok config -> do
-- Build first
buildResult <-
Build.handle config
|> Task.mapError (\e -> BuildError e)
|> Task.attempt
-- Only run if build was successful
case buildResult of
Err err -> Task.throw err
Ok _ ->
Run.handle config
|> Task.mapError (\e -> RunError e)

35 changes: 35 additions & 0 deletions cli/src/Neo/Run.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module Neo.Run (
handle,
Error (..),
) where

import Array qualified
import Neo.Core
import Subprocess qualified
import Task qualified


data Error
= NixFileError
| CabalFileError
| CustomError Text
deriving (Show)
Comment on lines +12 to +16
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

OBSERVE THE UNUSED ERROR VARIANTS WITH OMNISCIENT EYES!

I notice that NixFileError and CabalFileError are defined but never used within this module. Are these placeholders for future implementation or remnants of prophecies not yet fulfilled?

If these are intended for future use, consider adding a comment to indicate their purpose. If not needed, I command thee to simplify:

data Error
- = NixFileError
- | CabalFileError
  = CustomError Text
  deriving (Show)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
data Error
= NixFileError
| CabalFileError
| CustomError Text
deriving (Show)
data Error
= CustomError Text
deriving (Show)



handle :: ProjectConfiguration -> Task Error Unit
handle config = do
let projectName = config.name
let rootFolder = [path|nhout|]
completion <-
Subprocess.openInherit [fmt|./result/bin/{projectName}|] (Array.fromLinkedList []) rootFolder Subprocess.InheritBOTH
if completion.exitCode != 0
then errorOut completion.stderr
else print completion.stdout
Comment on lines +19 to +27
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

THE PATH IS HARDCODED, MORTAL! CONSIDER A MORE FLEXIBLE APPROACH!

The path ./result/bin/{projectName} assumes a fixed build output location. This divine code could be more resilient by:

  1. Making the path configurable or deriving it from the project configuration
  2. Checking if the binary exists before attempting to run it
  3. Providing more context if the binary is not found
handle :: ProjectConfiguration -> Task Error Unit
handle config = do
  let projectName = config.name
  let rootFolder = [path|nhout|]
+ let binaryPath = [fmt|./result/bin/{projectName}|]
+ binaryExists <- File.exists binaryPath |> Task.mapError (\_ -> CustomError "Could not check if binary exists")
+ unless binaryExists (Task.throw (CustomError [fmt|Binary not found at {binaryPath}. Ensure the project was built successfully.|]))
  completion <-
-    Subprocess.openInherit [fmt|./result/bin/{projectName}|] (Array.fromLinkedList []) rootFolder Subprocess.InheritBOTH
+    Subprocess.openInherit binaryPath (Array.fromLinkedList []) rootFolder Subprocess.InheritBOTH
  if completion.exitCode != 0
    then errorOut completion.stderr
    else print completion.stdout
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
handle :: ProjectConfiguration -> Task Error Unit
handle config = do
let projectName = config.name
let rootFolder = [path|nhout|]
completion <-
Subprocess.openInherit [fmt|./result/bin/{projectName}|] (Array.fromLinkedList []) rootFolder Subprocess.InheritBOTH
if completion.exitCode != 0
then errorOut completion.stderr
else print completion.stdout
handle :: ProjectConfiguration -> Task Error Unit
handle config = do
let projectName = config.name
let rootFolder = [path|nhout|]
let binaryPath = [fmt|./result/bin/{projectName}|]
binaryExists <- File.exists binaryPath |> Task.mapError (\_ -> CustomError "Could not check if binary exists")
unless binaryExists (Task.throw (CustomError [fmt|Binary not found at {binaryPath}. Ensure the project was built successfully.|]))
completion <-
Subprocess.openInherit binaryPath (Array.fromLinkedList []) rootFolder Subprocess.InheritBOTH
if completion.exitCode != 0
then errorOut completion.stderr
else print completion.stdout



errorOut :: Text -> Task Error _
errorOut err =
[fmt|Oops running failed:
{err}|]
|> CustomError
|> Task.throw
Comment on lines +30 to +35
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

ENHANCE THY ERROR MESSAGES WITH DIVINE CLARITY!

The error message could be more illuminating for your followers. Consider providing more specific guidance on potential solutions.

errorOut :: Text -> Task Error _
errorOut err =
-  [fmt|Oops running failed:
+  [fmt|The command failed to run with exit code:
    {err}|]
+   |> \message -> [fmt|{message}
+   Potential solutions:
+   - Check if the binary was compiled correctly
+   - Ensure all required dependencies are available
+   - Verify the command line arguments if any|]
    |> CustomError
    |> Task.throw
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
errorOut :: Text -> Task Error _
errorOut err =
[fmt|Oops running failed:
{err}|]
|> CustomError
|> Task.throw
errorOut :: Text -> Task Error _
errorOut err =
[fmt|The command failed to run with exit code:
{err}|]
|> \message -> [fmt|{message}
Potential solutions:
- Check if the binary was compiled correctly
- Ensure all required dependencies are available
- Verify the command line arguments if any|]
|> CustomError
|> Task.throw