invertase / melos

๐ŸŒ‹ A tool for managing Dart projects with multiple packages. With IntelliJ and Vscode IDE support. Supports automated versioning, changelogs & publishing via Conventional Commits.
https://melos.invertase.dev/~melos-latest
Apache License 2.0
1.13k stars 201 forks source link

fix: tryParse line-length to int when it's not already an integer #708

Closed jkoenig134 closed 5 months ago

jkoenig134 commented 5 months ago

Description

The flag --line-length was added, but it seems like it doesn't work as ArgParser doesn't seem to transform this param to int by default. This fix takes the String argument and tries parsing it to an int.

Type of Change

CLAassistant commented 5 months ago

CLA assistant check
All committers have signed the CLA.

jkoenig134 commented 5 months ago

@spydon I think the main issues why this occurred is that #689 didn't add a test. For protecting this command for regression issues I'm totally with you, I am open looking into the testing and adding tests for this.

utamori commented 5 months ago

@jkoenig134 thank you! I tried to add a test, but it didn't work.

I just made the following modification and was able to confirm that it works correctly at hand.

    final lineLength =
        switch (int.tryParse(argResults?['line-length'] as String)) {
      final int length => length,
      _ => null,
    };
melos activate
melos foramt --line-length 120 --set-exit-if-changed
jkoenig134 commented 5 months ago

@spydon added the test

jkoenig134 commented 5 months ago

@spydon as a follow-up i would really like to add setExitIfChanged and lineLength to the config.

I am mostly though with that. As it is not only a Bugfix - can I PR this?

spydon commented 5 months ago

@spydon as a follow-up i would really like to add setExitIfChanged and lineLength to the config.

I am mostly though with that. As it is not only a Bugfix - can I PR this?

Sure, sounds good!