ignite-cli icon indicating copy to clipboard operation
ignite-cli copied to clipboard

feat: Integrate mason_logger for improved logging and user prompts

Open Luckey-Elijah opened this issue 9 months ago • 11 comments

I tried to decrease the imprint of any API changes (specifically, those in utils.dart). But I think it would be ideal if we could also use the other APIs of mason_logger for writing to stdout/stderr (err/info/detail/etc) methods.

should fix #12 and #14

Luckey-Elijah avatar Apr 19 '25 01:04 Luckey-Elijah

CC: @luanpotter @spydon What are your thoughts on converting this to a more conventional args Command/CommandRunner application? It would look something like this:

Future<void> mainCommand(List<String> args) async {
  await FlameVersionManager.init();
  final runner = IgniteCommandRunner(
    logger: Logger(),
    flameVersionManager: FlameVersionManager.singleton,
  );

  exit((await runner.run(args)).code);
}
class IgniteCommandRunner extends CommandRunner<ExitCode> {
  IgniteCommandRunner({
    required Logger logger,
    required FlameVersionManager flameVersionManager,
  })  : _logger = logger,
        super(_name, _description) {
    addCommand(
      CreateCommand(
        logger: logger,
        flameVersionManager: flameVersionManager,
      ),
    );
    argParser.addFlag(
      'version',
      abbr: 'v',
      help: 'Print the current version.',
      negatable: false,
    );
  }
}
class CreateCommand extends Command<ExitCode> {
  CreateCommand({
    required Logger logger,
    required FlameVersionManager flameVersionManager,
  })  : _logger = logger,
        _manager = flameVersionManager,
        super() {
    final packages = flameVersionManager.versions;
    final flameVersions = packages[Package.flame]!;

    argParser
      ..addFlag(
        'interactive',
        abbr: 'i',
        help: 'Whether to run in interactive mode or not.',
        defaultsTo: true,
      )
      ..addOption(
        'name',
        help: 'The name of your game (valid dart identifier).',
      )
      ..addOption(
        'org',
        help: 'The org name, in reverse domain notation '
            '(package name/bundle identifier).',
      )
      ..addFlag(
        'create-folder',
        abbr: 'f',
        help: 'If you want to create a new folder on the current location with '
            "the project name or if you are already on the new project's folder.",
      )
      ..addOption(
        'template',
        help: 'What Flame template you would like to use for your new project',
        allowed: ['simple', 'example'],
      )
      ..addOption(
        'flame-version',
        help: 'What Flame version you would like to use.',
        allowed: [
          ...flameVersions.versions.take(5),
          '...',
          flameVersions.versions.last,
        ],
      )
      ..addMultiOption(
        'extra-packages',
        help: 'Which packages to use',
        allowed: packages.keys.map((e) => e.name).toList(),
      );
  }
}

Luckey-Elijah avatar Apr 23 '25 15:04 Luckey-Elijah

What are your thoughts on converting this to a more conventional args Command/CommandRunner application?

What would the differences be? The code definitely looks cleaner.

spydon avatar Apr 23 '25 15:04 spydon

The Command/CommandRunner setup handles usages like -h/--help for you. And it a bit more clearer to future contributors for add a new flag, option, or behavior to a specific command (or even adding a brand new command). And as you pointed out, it keeps the configuration/ArgParser of a given command more "scoped" to that command

Luckey-Elijah avatar Apr 23 '25 15:04 Luckey-Elijah

If I'm understanding it correctly, this is quite similar to what we have, just uses the Command class for each command and moves the "setup" logic inside each command, correct? (eg these lines, that already exist, are just move inside the command)

we would still need to keep all the code for the interactive mode, and the functionality would be exactly the same?

if so - definitely, let's do it!

luanpotter avatar Apr 24 '25 09:04 luanpotter

maybe we can also create an IgniteContext class that wraps

    required Logger logger,
    required FlameVersionManager flameVersionManager,

similar to my idea above? then all commands can receive the same parameter and we can easily add more injected helpers later on w/o modifying signatures

luanpotter avatar Apr 24 '25 09:04 luanpotter

maybe we can also create an IgniteContext class that wraps

Yes! I will add something that can encapsulate those (and future) objects

Luckey-Elijah avatar Apr 25 '25 16:04 Luckey-Elijah

I am wrapping up some tests now that I am at a more comfortable spot for how the APIs are setup. The IgniteContext right now is going to be the object used to get services (manager and version manager) and do some actions (execute Process.run and mason bundler actions). Basically to we can mock it's members.

Luckey-Elijah avatar Apr 29 '25 03:04 Luckey-Elijah

I haven't forgot about this 😅. I working on a feature for https://github.com/felangel/mason/issues/1550 first then going to integrate that new feature here to bring it back to parity

Luckey-Elijah avatar Jun 08 '25 19:06 Luckey-Elijah

@Luckey-Elijah is this something I could help out with? is it blocked on a mason PR that I can help expedite, or require some implementation there that we can figure out, or can we merge an initial version and then followup with @felangel later?

happy to help with whatever is needed to get this through, such a huge improvement!

luanpotter avatar Sep 27 '25 22:09 luanpotter

Hey @luanpotter thank you for following up. Due to personal obligations recently I cannot complete this in a timely manner. Feel free to use the work I've done in my branch if youu'd like to take over this effort

Luckey-Elijah avatar Oct 08 '25 20:10 Luckey-Elijah

@Luckey-Elijah no problem at all, totally understand :) I might try to take it over in the following weeks, or - I added the hacktoberfest label maybe some kind soul elsewhere will be interested as well

luanpotter avatar Oct 08 '25 21:10 luanpotter