Skip to content
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.

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

Tried to add multiple arguments to options #33

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

dXu23
Copy link
Contributor

@dXu23 dXu23 commented Mar 10, 2022

Closes #15

So far the following greet.ts appears to work

import * as Line from "./mod.ts";

class GreetMainCommand extends Line.MainCommand {
  public signature = "greet [greeting] [name]"

  public arguments = {
    "greeting": "The greeting before the name.",
    "name": "The name of the thing to greet.",
  };

  public options = {
    "--some-option [arg_1] [arg_2]": "description",
  };

  public handle(): void {
    const someOption = this.option("--some-option");

    if (someOption) {
      console.log(`someOption: ${someOption}`);
    }

    const greeting = this.argument("greeting");
    const name = this.argument("name");

    console.log(`${greeting}, ${name}!`);
  }
}

const cli = new Line.CLI({
  name: "Greeter CLI",
  description: "A CLI that outputs greets",
  version: "v1.0.0",
  command: GreetMainCommand,
});

cli.run();
> deno run greet.ts --some-option 3 4 hello world
[debug output]
someOption: 3,4
hello, world

Changes not final and tests need to be written. It might be better to use regexes here.

@dXu23 dXu23 marked this pull request as ready for review March 11, 2022 19:56
@dXu23 dXu23 changed the title WIP: Tried to add multiple arguments to options Tried to add multiple arguments to options Mar 26, 2022
@crookse
Copy link
Member

crookse commented Apr 8, 2022

Sorry @dXu23, didn't see that you made this ready for review. however im kind of confused because the code has commented out console log statements. still working on this?

@dXu23
Copy link
Contributor Author

dXu23 commented Apr 10, 2022

Sorry about that. I kind of forgot to remove the console.log statements.

let lastOptionObject: IOption | null = null;
const optionsProcessed: string[] = [];

passedInOptions.forEach((option: string) => {
Copy link
Member

Choose a reason for hiding this comment

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

@dXu23 , again sorry for being late. don't get much time to look at the lower priority modules. anyways, any reason you went with different loops as opposed to keeping the .forEeach here? i ask because from a readability point of view, it's difficult for me to keep track of what's being iterated. im all for different loops, but reason i chose for each was so that i didn't have to add [i] to retrieve values. wondering if you found this approach to be better

optionObject.takes_value = true;
const sig = signature.substring(0, openBracketNdx).trim();
Copy link
Member

@crookse crookse Jun 19, 2022

Choose a reason for hiding this comment

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

question here. what's the reason behind tokenizing the string in this else block? is it mainly for validating that the option is written properly so it can be parsed? simply trying to get context here so i can better understand the goal and if there may be another way than tokenizing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Add support for N amount of values for options
2 participants