Deion create a new --indent-

📎 Deprecate the CLI option `--indent-size` and create `--indent-width` about biome HOT 15 CLOSED

biomejs commented on May 19, 2024

📎 Deprecate the CLI option `--indent-size` and create `--indent-width`

from biome.

Comments (15)

ematipico commented on May 19, 2024 1

Should I create an array to store all the deprecated arguments, in order to leave it generic for future uses?

It's possible that in the future we don't deprecate an option for another, that's why a generic message is more future proof

from biome.

ematipico commented on May 19, 2024 1

Oh, I guess the previous windows users didn't use this command 😔 not sure how to fix that, I don't use windows

You can still write a test, they are very reliable, and we have many of them

from biome.

ematipico commented on May 19, 2024 1

What would be the best or correct way to add this option? I tried putting it in the formatter configuration in the rome_service crate, but that breaks tests that apparently don't use the indent-size option through the CLI.

That's the correct place. I'm not sure what you mean when tests break, maybe the best option is opening a draft PR so we can look a it, when code at hand.

from biome.

ematipico commented on May 19, 2024 1

I understand now. Still, it would be great to see some code, so I can better understand the implementation, and give suggestions. It's fine there aren't any tests, that's what draft PRs are also good for: get feedback

from biome.

ematipico commented on May 19, 2024 1

width is more in line with the meaning of the word given its context, because it's applied on a line. And it's also in line with line_width.

from biome.

Levieber commented on May 19, 2024

I will try to do this

from biome.

Levieber commented on May 19, 2024

Should I use/create a diagnostic struct for this? Where should I check the provided option?

I'm thinking of doing based on #107 if you have any tips better.

from biome.

ematipico commented on May 19, 2024

Should I use/create a diagnostic struct for this? Where should I check the provided option?

Yes, you'll have to create a new diagnostic.

biome/crates/rome_cli/src/diagnostics.rs

Lines 23 to 60 in ef3af45

 pub enum CliDiagnostic { 

 /// Returned when it is called with a subcommand it doesn't know 

 UnknownCommand(UnknownCommand), 

 /// Return by the help command when it is called with a subcommand it doesn't know 

 UnknownCommandHelp(UnknownCommandHelp), 

 /// Returned when the value of a command line argument could not be parsed 

 ParseError(ParseDiagnostic), 

 /// Returned when the CLI doesn't recognize a command line argument 

 UnexpectedArgument(UnexpectedArgument), 

 /// Returned when a required argument is not present in the command line 

 MissingArgument(MissingArgument), 

 /// Returned when a subcommand is called without any arguments 

 EmptyArguments(EmptyArguments), 

 /// Returned when a subcommand is called with an unsupported combination of arguments 

 IncompatibleArguments(IncompatibleArguments), 

 /// Returned by a traversal command when error diagnostics were emitted 

 CheckError(CheckError), 

 /// Emitted when a file is fixed, but it still contains diagnostics. 

 /// 

 /// This happens when these diagnostics come from rules that don't have a code action. 

 FileCheck(FileCheck), 

 /// When an argument is higher than the expected maximum 

 OverflowNumberArgument(OverflowNumberArgument), 

 /// Wrapper for an underlying `rome_service` error 

 WorkspaceError(WorkspaceError), 

 /// Wrapper for an underlying `std::io` error 

 IoError(IoDiagnostic), 

 /// The daemon is not running 

 ServerNotRunning(ServerNotRunning), 

 /// The end configuration (`biome.json` + other options) is incompatible with the command 

 IncompatibleEndConfiguration(IncompatibleEndConfiguration), 

 /// No files processed during the file system traversal 

 NoFilesWereProcessed(NoFilesWereProcessed), 

 /// Errors thrown when running the `biome migrate` command 

 MigrateError(MigrationDiagnostic), 

 /// When the VCS folder couldn't be found 

 NoVcsFolderFound(NoVcsFolderFound), 

 }

You could create a generic DeprecatedArgument inside this enum, that accepts a generic message. Or a loose diagnostic like in #107. This the latter is a better solution

I'm thinking of doing based on #107 if you have any tips better.

Yes that's right!

from biome.

Levieber commented on May 19, 2024

Should I create an array to store all the deprecated arguments, in order to leave it generic for future uses?

from biome.

Levieber commented on May 19, 2024

When I run the CLI command (cargo run --bin biome -- --help), it gives the following error: thread 'main' has overflowed its stack.

I'm using Windows.

from biome.

ematipico commented on May 19, 2024

To run the CLI, do

cargo biome-cli-dev --help

from biome.

Levieber commented on May 19, 2024

I tested this command now, and it also gave the same error.

from biome.

Levieber commented on May 19, 2024

I'm sorry I didn't ask for this information sooner

What would be the best or correct way to add this option? I tried putting it in the formatter configuration in the rome_service crate, but that breaks tests that apparently don't use the indent-size option through the CLI.

I checked the deprecated option in the or_diagnostic function equal to #107.

Is there anything I forgot to do or did wrong?

from biome.

Levieber commented on May 19, 2024

Explaining better: the way I used to check the indent-size option (very similar to #107) apparently takes the settings from the configuration file, which causes the diagnostic to be run even without pass the deprecated CLI option.

If you still don't understand, I committed an early version (without tests) of what I tried to do, and I can make a draft pull request.

from biome.

Conaclos commented on May 19, 2024

I first thought this was a good idea. However, I am starting to wonder if it is still. Prettier and .editorConfig use indent_size.

from biome.

📎 Deprecate the CLI option `--indent-size` and create `--indent-width` about biome HOT 15 CLOSED

Comments (15)

Related Issues (20)

Recommend Projects

React

Vue.js

Typescript

TensorFlow

Django

Laravel

D3

Recommend Topics

javascript

web

server

Machine learning

Visualization

Game

Recommend Org

Facebook

Microsoft

Google

Alibaba

D3

Tencent

	pub enum CliDiagnostic {
	/// Returned when it is called with a subcommand it doesn't know
	UnknownCommand(UnknownCommand),
	/// Return by the help command when it is called with a subcommand it doesn't know
	UnknownCommandHelp(UnknownCommandHelp),
	/// Returned when the value of a command line argument could not be parsed
	ParseError(ParseDiagnostic),
	/// Returned when the CLI doesn't recognize a command line argument
	UnexpectedArgument(UnexpectedArgument),
	/// Returned when a required argument is not present in the command line
	MissingArgument(MissingArgument),
	/// Returned when a subcommand is called without any arguments
	EmptyArguments(EmptyArguments),
	/// Returned when a subcommand is called with an unsupported combination of arguments
	IncompatibleArguments(IncompatibleArguments),
	/// Returned by a traversal command when error diagnostics were emitted
	CheckError(CheckError),
	/// Emitted when a file is fixed, but it still contains diagnostics.
	///
	/// This happens when these diagnostics come from rules that don't have a code action.
	FileCheck(FileCheck),
	/// When an argument is higher than the expected maximum
	OverflowNumberArgument(OverflowNumberArgument),
	/// Wrapper for an underlying `rome_service` error
	WorkspaceError(WorkspaceError),
	/// Wrapper for an underlying `std::io` error
	IoError(IoDiagnostic),
	/// The daemon is not running
	ServerNotRunning(ServerNotRunning),
	/// The end configuration (`biome.json` + other options) is incompatible with the command
	IncompatibleEndConfiguration(IncompatibleEndConfiguration),
	/// No files processed during the file system traversal
	NoFilesWereProcessed(NoFilesWereProcessed),
	/// Errors thrown when running the `biome migrate` command
	MigrateError(MigrationDiagnostic),
	/// When the VCS folder couldn't be found
	NoVcsFolderFound(NoVcsFolderFound),
	}