Wave-Play / robo.js

Power up Discord with effortless activities, bots, servers, and more! ⚡
https://robojs.dev
MIT License
43 stars 22 forks source link

Make multi-arg options optional #334

Open Rishi-0007 opened 1 week ago

Rishi-0007 commented 1 week ago

Summary:

This PR fixes issue #331, where single-value options in the CLI handler were incorrectly consuming positional arguments. The parser now correctly distinguishes between single-value and multi-value options, ensuring options like -k or --kit consume only one argument, while options like --plugins can accept multiple values.

Key Changes:

Examples of Corrected Behavior:

Impact:

Testing:

Notes:


Thank you for reviewing this PR.

vercel[bot] commented 1 week ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
robo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 28, 2024 8:37pm
Pkmmte commented 1 week ago

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

Rishi-0007 commented 1 week ago

I have read the CLA Document and I hereby sign the CLA

Pkmmte commented 1 week ago

Thank you for this! Can you please verify that multi-arg options are still possible? If so, how does the API distinguish between them?

For example, a --plugins option should still be able to accept multiple values. (e.g. "--plugins @robojs/ai @robojs/sync")

Rishi-0007 commented 1 week ago

Thank you for this! Can you please verify that multi-arg options are still possible? If so, how does the API distinguish between them?

For example, a --plugins option should still be able to accept multiple values. (e.g. "--plugins @robojs/ai @robojs/sync")

Hey @Pkmmte , I've made the changes based on your feedback. Please check it and let me know if this approach is good or any further changes are required.

Rishi-0007 commented 1 week ago

Hi @Pkmmte,

Thank you for your feedback and for accepting the PR for Hacktoberfest. I’ve been working on addressing the issues mentioned, specifically ensuring options like -v after positional arguments are correctly parsed and not treated as file paths.

Here’s what I’ve tried so far:

  1. Adjusted parseOptions in cli-handler.ts: Refined the parsing logic to separate options from positional arguments and ensure options with spaces return single strings by default.
  2. Verified Argument Parsing with Tests: Added and confirmed tests for scenarios with options appearing after positional arguments, ensuring options are recognized correctly.
  3. Encountering Persistent ENOENT Error: When running the command:

    npx robo build src/cli/commands/add.ts -v

    the -v flag continues to be interpreted as a positional argument, causing an attempt to locate a file at -v, and resulting in the ENOENT error. This behavior appears on both the main branch and my modified branch, suggesting it may be due to the overall handling of positional arguments in the build command.

Would you be able to provide additional guidance on addressing this issue? Specifically, are there any expected parsing behaviors or modifications within cli-handler.ts that would prevent options like -v from being treated as positional arguments in the context of the build command?

Thank you for your time and assistance.

Nazeofel commented 1 week ago

Hello ! thanks for your contribution, I have just tested it out, and it seems to be working just fine for me, are you sure to be testing out the right branch ?

Rishi-0007 commented 1 week ago

Hello ! thanks for your contribution, I have just tested it out, and it seems to be working just fine for me, are you sure to be testing out the right branch ?

Hi @Nazeofel, Thanks for testing it out. Is this the expected output or am I doing something wrong?

npx robo build src/cli/commands/add.ts -v
info  - Building Robo...
node:internal/fs/promises:1032
  const result = await PromisePrototypeThen(
                 ^

Error: ENOENT: no such file or directory, stat '/home/rishi/code/robo.js/packages/robo/-v'
    at async Object.stat (node:internal/fs/promises:1032:18)
    at async traverse (file:///home/rishi/code/robo.js/packages/robo/dist/cli/utils/compiler.js:59:1
8)                                                                                                      at async Object.buildCode (file:///home/rishi/code/robo.js/packages/robo/dist/cli/utils/compiler
.js:133:3)                                                                                              at async Command.buildAction [as _handler] (file:///home/rishi/code/robo.js/packages/robo/dist/c
li/commands/build/index.js:63:23)                                                                       at async Command.processSubCommand (file:///home/rishi/code/robo.js/packages/robo/dist/cli/utils
/cli-handler.js:255:5)                                                                                  at async Command.processSubCommand (file:///home/rishi/code/robo.js/packages/robo/dist/cli/utils
/cli-handler.js:245:9) {                                                                              errno: -2,
  code: 'ENOENT',
  syscall: 'stat',
  path: '/home/rishi/code/robo.js/packages/robo/-v'
}

Node.js v22.9.0

But when I removed -v, I got this output(which is not an error I guess):

npx robo build src/cli/commands/add.ts   
info  - Building Robo...

Type                Name            Description
───────────────────────────────────────────────
Command            Δ /help            Displays a list of commands.
Event              Δ interactionCreate
Event              Δ ready  

Δ = Automatically generated

Robo size: 836.92 kB
Built in 113ms

discord:error - DISCORD_TOKEN or DISCORD_CLIENT_ID not found in environment variables

Is this what you are talking about in the previous comment @Pkmmte?

Rishi-0007 commented 1 week ago

Hey @Pkmmte,

I’ve tested the issue on the main branch as well and observed the following outputs in two cases:

This ENOENT error seems unrelated to my changes since it’s happening on the main branch as well. Could you confirm if I’m missing something or if there’s another approach I should take? Your guidance would be greatly appreciated.

Thank you!

Nazeofel commented 1 week ago

Hey ! it should work just fine on the main branch and latest version of Robo, just to confirm somethin, which OS are you running ?

Rishi-0007 commented 1 week ago

Hey ! it should work just fine on the main branch and latest version of Robo, just to confirm somethin, which OS are you running ?

Ubuntu noble 24.04 x86_64

Nazeofel commented 6 days ago

Hey sorry to get back to you so late, we cannot reproduce this issue at all on our environment with the main branch, tested on MacOS, Windows, and Linux too. would you be able to share your all Linux configuration where you are running the command please.