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

Read air.toml and use in air format #109

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DavisVaughan
Copy link
Collaborator

@DavisVaughan DavisVaughan commented Dec 16, 2024

Closes #83

A first pass of support for air.toml

At a high level:

  • air.toml supported file structure is outlined by the TomlOptions struct. Every field in this struct is Option<> guarded.

  • TomlOptions::into_settings() converts a TomlOptions into a finalized Settings, using defaults when nothing was set. Settings is more friendly for internal use than for user facing structure.

  • With air format dir1 dir2, for both dir1 and dir2 we find the "closest" air.toml for each path and save the directory it was found in as, say, ancestor1 and ancestor2. We read in the air.toml and associate the Settings with that ancestor directory using a Resolver.

  • We iterate through dir1 and dir2 collecting a flat vector of R files. Then we iterate over each R file path, retrieve its associated Settings from the Resolver, and format with that path + settings combination.

This is only set up for the CLI right now. I imagine for the LSP we can preload a Resolver with Workspace paths the LSP provides us as it starts up. I'd like to try to implement this before we merge this to make sure we have the right abstractions in place.

Comment on lines +138 to +149
// TODO: Make these configurable options (possibly just one?)
// Right now we explicitly call them even though they are `true` by default
// to remind us to expose them.
//
// "This toggles, as a group, all the filters that are enabled by default"
// builder.standard_filters(true)
builder.hidden(true);
builder.parents(true);
builder.ignore(true);
builder.git_ignore(true);
builder.git_global(true);
builder.git_exclude(true);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's nice that we will get gitignore support out of the box so we don't format any directories marked in a .gitignore

Comment on lines +3 to +5
// TODO: Can we pick a better crate name for `line_ending` so these don't collide?
#[path = "settings/line_ending.rs"]
mod line_ending_setting;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mildly annoying. We have a line_ending crate but I've also added settings/line_ending.rs as a mod and I need to use both in here 😢 .

Comment on lines +37 to +60
impl FormatSettings {
// Finalize `RFormatOptions` in preparation for a formatting operation on `source`
pub fn to_format_options(&self, source: &str) -> RFormatOptions {
let line_ending = match self.line_ending {
LineEnding::Lf => biome_formatter::LineEnding::Lf,
LineEnding::Crlf => biome_formatter::LineEnding::Crlf,
#[cfg(target_os = "windows")]
LineEnding::Native => biome_formatter::LineEnding::Crlf,
#[cfg(not(target_os = "windows"))]
LineEnding::Native => biome_formatter::LineEnding::Lf,
LineEnding::Auto => match line_ending::infer(source) {
line_ending::LineEnding::Lf => biome_formatter::LineEnding::Lf,
line_ending::LineEnding::Crlf => biome_formatter::LineEnding::Crlf,
},
};

RFormatOptions::new()
.with_indent_style(self.indent_style.into())
.with_indent_width(self.indent_width.into())
.with_line_ending(line_ending)
.with_line_width(self.line_length.into())
.with_magic_line_break(self.magic_line_break.into())
}
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Finalized FormatSettings are really "mostly" finalized. Things like LineEnding::Auto get truly finalized when converting to RFormatOptions, because that's when we are most likely to have the source code to infer line endings for lying around.

Comment on lines +3 to +5
#[derive(Copy, Clone, Debug, Eq, PartialEq, Default, serde::Deserialize)]
#[serde(rename_all = "kebab-case")]
pub enum LineEnding {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think it is very reasonable / inevitable that we end up with our own copies of these settings in addition to the ones from biome.

  • For each one we need a Deserialize method that works nicely with the toml crate and uses kebab-case
  • For some, like here, we add additional user facing options like Auto that don't make it into the underlying formatter option
  • For some, like IndentWidth, we may choose to have our own IndentWidth::default() that is different from what biome's is

Comment on lines +9 to +16
/// Parse an `air.toml` file.
pub fn parse_air_toml<P: AsRef<Path>>(path: P) -> Result<TomlOptions, ParseTomlError> {
let contents = std::fs::read_to_string(path.as_ref())
.map_err(|err| ParseTomlError::Read(path.as_ref().to_path_buf(), err))?;

toml::from_str(&contents)
.map_err(|err| ParseTomlError::Deserialize(path.as_ref().to_path_buf(), err))
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the entire set of parsing code. It's wild how little it is. Everything is set up at compile time instead!

Comment on lines +33 to +40
/// The line length at which the formatter prefers to wrap lines.
///
/// The value must be greater than or equal to `1` and less than or equal to `320`.
///
/// Note: While the formatter will attempt to format lines such that they remain
/// within the `line-length`, it isn't a hard upper bound, and formatted lines may
/// exceed the `line-length`.
pub line_length: Option<LineLength>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ruff has some very cool macros that use TomlOptions as its "source of truth" for documentation.

It extracts out each table name (like format) and each option under the table (like line_length) along with all documentation comments and makes a really nice markdown page for it that ends up here:

https://docs.astral.sh/ruff/settings/

Without a page like this its hard to know what the spec is for what you can supply in air.toml

/// This option changes the number of spaces the formatter inserts when
/// using `indent-style = "space"`. It also represents the width of a tab when
/// `indent-style = "tab"` for the purposes of computing the `line-length`.
pub indent_width: Option<IndentWidth>,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Both line_length and indent_width are top-level global options used by "all of air" because if we consider a linter then it will need access to these options to determine if a line exceeds the line length.

Compare that with indent_style, line_ending, and ignore_magic_line_break which are more clearly format specific and live under the [format] table header.

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

Successfully merging this pull request may close these issues.

Add a basic configuration parser for air.toml
1 participant