CF-495 Codeflash init - check if the formatter is installed #21
Conversation
codeflash/cli_cmds/cmd_init.py
Outdated
| click.echo(f"✅ Formatter exists on system") | ||
| click.echo() | ||
| except FileNotFoundError as e: | ||
| click.echo(f"⚠️ Formatter not found: {formatter}") |
There was a problem hiding this comment.
add a helpful message to help the user resolve the problem. Suggest installing it or ensuring it is present
codeflash/cli_cmds/cmd_init.py
Outdated
| click.echo(f"⚠️ Formatter not found: {formatter}") | ||
| click.echo() | ||
| # Not throwing an exception, letting the program proceed even though the formatter was not found, putting it on the user to install it later | ||
| # raise e from None |
codeflash/cli_cmds/cmd_init.py
Outdated
| if formatter in ["black", "ruff"]: | ||
| try: | ||
| result = subprocess.run([formatter], capture_output=True, check=False) | ||
| click.echo(f"✅ Formatter exists on system") |
There was a problem hiding this comment.
we might not need to print this, we want to reduce the cognitive load for something thats not needed
codeflash/cli_cmds/cmd_init.py
Outdated
| try: | ||
| result = subprocess.run([formatter], capture_output=True, check=False) | ||
| click.echo(f"✅ Formatter exists on system") | ||
| click.echo() |
There was a problem hiding this comment.
what does this blank line do? is this necessary?
| @@ -587,6 +587,11 @@ def configure_pyproject_toml(setup_info: SetupInfo) -> None: | |||
| ) | |||
| elif formatter == "don't use a formatter": | |||
| formatter_cmds.append("disabled") | |||
| if formatter in ["black", "ruff"]: | |||
| try: | |||
| result = subprocess.run([formatter], capture_output=True, check=False) | |||
There was a problem hiding this comment.
nit: shouldnt we use shutil.which to check if its in file path rather than running the process?
And we might have to test this in other OS for consistency.
There was a problem hiding this comment.
we want to test for existence the same way it is run later for formatting. To reduce any inconsistencies
There was a problem hiding this comment.
choices=["black", "ruff", "other", "don't use a formatter"],
in here
we allow for other as a choice, won't this break that behavior?
Not throwing an exception, letting the program proceed even though the formatter was not found, putting it on the user to install it later. Or we can ask to install it for them.