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

feat(format): respect max line width #242

Merged
merged 47 commits into from
Jan 15, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
e27f90c
[WIP] match current tests while using a buffer
a-frantz Oct 20, 2024
f74d717
[WIP]
a-frantz Oct 22, 2024
164261a
Update builder.rs
a-frantz Oct 22, 2024
0f691d7
feat: config max line length
a-frantz Oct 22, 2024
c364a45
Update max_line_length.rs
a-frantz Oct 22, 2024
2852d28
revise: indentation config
a-frantz Oct 22, 2024
165603a
Update post.rs
a-frantz Oct 23, 2024
f1f4dec
Merge branch 'main' into feat/line-width
a-frantz Oct 30, 2024
f2d6448
Merge branch 'main' into feat/line-width
a-frantz Oct 31, 2024
ccc1104
WIP
a-frantz Oct 31, 2024
e1898e5
Merge branch 'main' into feat/line-width
a-frantz Jan 2, 2025
af36d91
WIP
a-frantz Jan 2, 2025
ce3a454
revise: use constants for min and max max line len
a-frantz Jan 2, 2025
3278597
revise: use a constant for max space indent
a-frantz Jan 2, 2025
95d649c
revise: hardcode tabs and spaces in fewer places
a-frantz Jan 2, 2025
163237e
chore: code clean up
a-frantz Jan 6, 2025
283ae6a
WIP
a-frantz Jan 6, 2025
df6f7a2
WIP
a-frantz Jan 6, 2025
a238b95
WIP
a-frantz Jan 6, 2025
a574f09
chore: consistency
a-frantz Jan 13, 2025
bc9f288
Merge branch 'main' into feat/line-width
a-frantz Jan 13, 2025
caa29c8
chore: code cleanup
a-frantz Jan 13, 2025
7509e66
revise: new PostToken (TempIndent)
a-frantz Jan 13, 2025
3dc09c6
fix: handle edge case where stream is empty
a-frantz Jan 13, 2025
7c0e191
WIP
a-frantz Jan 13, 2025
259ea59
perf: use Rc for temp_indent
a-frantz Jan 13, 2025
a34c9e0
WIP
a-frantz Jan 13, 2025
7199199
chore: fmt
a-frantz Jan 13, 2025
2e77ba3
WIP
a-frantz Jan 14, 2025
8f51518
WIP
a-frantz Jan 14, 2025
6126627
chore: code clean up
a-frantz Jan 14, 2025
28643d2
fix: special handling for no line breaks
a-frantz Jan 14, 2025
f195a78
fix: reset indent level
a-frantz Jan 14, 2025
6bf613f
fix: simplify `IfExpr` formatting
a-frantz Jan 14, 2025
9fb4691
feat: can_be_line_broken is exhaustive
a-frantz Jan 15, 2025
51473d5
Merge branch 'main' into feat/line-width
a-frantz Jan 15, 2025
6049417
chore: code cleanup
a-frantz Jan 15, 2025
d2c683e
Merge branch 'feat/line-width' of https://github.com/stjude-rust-labs…
a-frantz Jan 15, 2025
d16da2a
Update CHANGELOG.md
a-frantz Jan 15, 2025
b4d1020
fix: rework Config Builder
a-frantz Jan 15, 2025
f55d392
Update builder.rs
a-frantz Jan 15, 2025
52a9984
chore: review feedback
a-frantz Jan 15, 2025
5a42020
Update wdl-format/src/token.rs
a-frantz Jan 15, 2025
c0e446d
revise: more specific name for line spacing policy
a-frantz Jan 15, 2025
b3a2631
chore: review feedback
a-frantz Jan 15, 2025
c62b9c1
chore: review feedback
a-frantz Jan 15, 2025
1c2b6c7
chore: review feedback
a-frantz Jan 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions wdl-format/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added

* Leading whitespace in command text is now normalized ([#240](https://github.com/stjude-rust-labs/wdl/pull/240)).
* Line breaks are now added in order to keep lines under the max line width (default 90 characters) ([#242](https://github.com/stjude-rust-labs/wdl/pull/242)).

### Fixed

Expand Down
13 changes: 11 additions & 2 deletions wdl-format/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,29 @@

mod builder;
mod indent;
mod max_line_length;

pub use builder::Builder;
pub use indent::Indent;
pub use max_line_length::MaxLineLength;

/// Configuration for formatting.
#[derive(Clone, Copy, Debug, Default)]
pub struct Config {
/// The number of characters to indent.
/// The indentation configuration.
indent: Indent,
/// The maximum line length.
max_line_length: MaxLineLength,
}

impl Config {
/// Gets the indent level of the configuration.
/// Gets the indentation configuration.
pub fn indent(&self) -> Indent {
self.indent
}

/// Gets the maximum line length of the configuration.
pub fn max_line_length(&self) -> Option<usize> {
self.max_line_length.get()
}
}
58 changes: 18 additions & 40 deletions wdl-format/src/config/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,65 +2,43 @@

use crate::Config;
use crate::config::Indent;

/// An error related to a [`Builder`].
#[derive(Debug)]
pub enum Error {
/// A required value was missing for a builder field.
Missing(&'static str),
}

impl std::fmt::Display for Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Error::Missing(field) => write!(
f,
"missing required value for '{field}' in a formatter configuration builder"
),
}
}
}

impl std::error::Error for Error {}

/// A [`Result`](std::result::Result) with an [`Error`].
pub type Result<T> = std::result::Result<T, Error>;
use crate::config::MaxLineLength;

/// A builder for a [`Config`].
#[derive(Default)]
pub struct Builder {
/// The number of characters to indent.
indent: Option<Indent>,
/// The maximum line length.
max_line_length: Option<MaxLineLength>,
}

impl Builder {
/// Creates a new builder with default values.
pub fn new() -> Self {
Default::default()
}

/// Sets the indentation level.
///
/// # Notes
///
/// This silently overwrites any previously provided value for the
/// indentation level.
pub fn indent(mut self, indent: Indent) -> Self {
self.indent = Some(indent);
self
}

/// Consumes `self` and attempts to build a [`Config`].
pub fn try_build(self) -> Result<Config> {
let indent = self.indent.ok_or(Error::Missing("indent"))?;

Ok(Config { indent })
/// Sets the maximum line length.
///
/// This silently overwrites any previously provided value for the maximum
/// line length.
pub fn max_line_length(mut self, max_line_length: MaxLineLength) -> Self {
a-frantz marked this conversation as resolved.
Show resolved Hide resolved
self.max_line_length = Some(max_line_length);
self
}
}

impl Default for Builder {
fn default() -> Self {
Self {
indent: Some(Default::default()),
/// Consumes `self` to build a [`Config`].
pub fn build(self) -> Config {
let indent = self.indent.unwrap_or_default();
let max_line_length = self.max_line_length.unwrap_or_default();
Config {
indent,
max_line_length,
}
}
}
61 changes: 56 additions & 5 deletions wdl-format/src/config/indent.rs
Original file line number Diff line number Diff line change
@@ -1,22 +1,73 @@
//! Indentation within formatting configuration.

use std::num::NonZeroUsize;
use crate::SPACE;
use crate::TAB;

/// The default number of spaces to represent one indentation level.
const DEFAULT_SPACE_INDENT: usize = 4;
/// The default indentation.
pub const DEFAULT_INDENT: Indent = Indent::Spaces(unsafe { NonZeroUsize::new_unchecked(4) });
pub const DEFAULT_INDENT: Indent = Indent::Spaces(DEFAULT_SPACE_INDENT);
/// The maximum number of spaces to represent one indentation level.
pub const MAX_SPACE_INDENT: usize = 16;

/// An indentation level.
#[derive(Clone, Copy, Debug)]
pub enum Indent {
/// Tabs.
Tabs(NonZeroUsize),

Tabs,
Copy link
Member

Choose a reason for hiding this comment

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

So if a user indents with tab, they can only use one tab. I'm fine with that, but just flagging that to make sure we're OK with that choice.

/// Spaces.
Spaces(NonZeroUsize),
Spaces(usize),
}

impl Default for Indent {
fn default() -> Self {
DEFAULT_INDENT
}
}

impl Indent {
/// Attempts to create a new indentation level configuration.
pub fn try_new(tab: bool, num_spaces: Option<usize>) -> Result<Self, String> {
match (tab, num_spaces) {
(true, None) => Ok(Indent::Tabs),
(true, Some(_)) => {
Err("Indentation with tabs cannot have a number of spaces".to_string())
}
(false, Some(n)) => {
if n > MAX_SPACE_INDENT {
Err(format!(
"Indentation with spaces cannot have more than {} characters",
MAX_SPACE_INDENT
))
} else {
Ok(Indent::Spaces(n))
}
}
(false, None) => Ok(Indent::Spaces(DEFAULT_SPACE_INDENT)),
}
}

/// Gets the number of characters to indent.
pub fn num(&self) -> usize {
match self {
Indent::Tabs => 1,
Indent::Spaces(n) => *n,
}
}

/// Gets the character used for indentation.
pub fn character(&self) -> &str {
match self {
Indent::Tabs => TAB,
Indent::Spaces(_) => SPACE,
}
}

/// Gets the string representation of the indentation.
pub fn string(&self) -> String {
match self {
Indent::Tabs => self.character().to_string(),
Indent::Spaces(n) => self.character().repeat(*n),
}
}
}
42 changes: 42 additions & 0 deletions wdl-format/src/config/max_line_length.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
//! Configuration for max line length formatting.

/// The default maximum line length.
pub const DEFAULT_MAX_LINE_LENGTH: usize = 90;
/// The minimum maximum line length.
pub const MIN_MAX_LINE_LENGTH: usize = 60;
/// The maximum maximum line length.
pub const MAX_MAX_LINE_LENGTH: usize = 240;
Comment on lines +3 to +8
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested any non-default line lengths?

Copy link
Member Author

Choose a reason for hiding this comment

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

Check out these 3 commits on workflows - stjudecloud/workflows@main...tmp/format-examples


/// The maximum line length.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct MaxLineLength(Option<usize>);

impl MaxLineLength {
/// Attempts to create a new `MaxLineLength` with the provided value.
///
/// A value of `0` indicates no maximum.
pub fn try_new(value: usize) -> Result<Self, String> {
let val = match value {
0 => Self(None),
MIN_MAX_LINE_LENGTH..=MAX_MAX_LINE_LENGTH => Self(Some(value)),
_ => {
return Err(format!(
"The maximum line length must be between {} and {} or 0",
MIN_MAX_LINE_LENGTH, MAX_MAX_LINE_LENGTH
));
}
};
Ok(val)
}

/// Gets the maximum line length. A value of `None` indicates no maximum.
pub fn get(&self) -> Option<usize> {
self.0
}
}

impl Default for MaxLineLength {
fn default() -> Self {
Self(Some(DEFAULT_MAX_LINE_LENGTH))
}
}
9 changes: 4 additions & 5 deletions wdl-format/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ pub const NEWLINE: &str = "\n";
/// A space.
pub const SPACE: &str = " ";

/// A tab.
pub const TAB: &str = "\t";

/// Returns exactly one entity from an enumerable list of entities (usually a
/// [`Vec`]).
#[macro_export]
Expand Down Expand Up @@ -208,16 +211,12 @@ impl Formatter {
}

/// Gets the [`PostToken`] stream.
///
/// # Notes
///
/// * This shouldn't be exposed publicly.
fn to_stream<W: Writable>(&self, element: W) -> TokenStream<PostToken> {
let mut stream = TokenStream::default();
element.write(&mut stream);

let mut postprocessor = Postprocessor::default();
postprocessor.run(stream)
postprocessor.run(stream, self.config())
}
}

Expand Down
45 changes: 35 additions & 10 deletions wdl-format/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ mod post;
mod pre;

use std::fmt::Display;
use std::rc::Rc;

pub use post::*;
pub use pre::*;
Expand All @@ -19,8 +20,7 @@ pub trait Token: Eq + PartialEq {
/// A stream of tokens. Tokens in this case are either [`PreToken`]s or
/// [`PostToken`]s. Note that, unless you are working on formatting
/// specifically, you should never need to work with [`PostToken`]s.
#[derive(Debug)]

#[derive(Debug, Clone)]
pub struct TokenStream<T: Token>(Vec<T>);

impl<T: Token> Default for TokenStream<T> {
Expand Down Expand Up @@ -52,6 +52,26 @@ impl<T: Token> TokenStream<T> {
let _ = self.0.pop();
}
}

/// Returns whether the stream is empty.
pub fn is_empty(&self) -> bool {
self.0.is_empty()
}

/// Returns an iterator over the tokens in the stream.
pub fn iter(&self) -> std::slice::Iter<'_, T> {
self.0.iter()
}

/// Clears the stream.
pub fn clear(&mut self) {
self.0.clear();
}

/// Extends the stream with the tokens from another stream.
pub fn extend(&mut self, other: Self) {
self.0.extend(other.0);
}
}

impl<T: Token> IntoIterator for TokenStream<T> {
Expand All @@ -64,28 +84,33 @@ impl<T: Token> IntoIterator for TokenStream<T> {
}

/// The kind of comment.
#[derive(Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Comment {
/// A comment on its own line.
Preceding(String),
Preceding(Rc<String>),
/// A comment on the same line as the code preceding it.
Inline(String),
Inline(Rc<String>),
}

/// Trivia.
#[derive(Debug, Eq, PartialEq)]
#[derive(Clone, Debug, Eq, PartialEq)]
pub enum Trivia {
/// A blank line. This may be ignored by the postprocessor.
BlankLine,
/// A comment.
Comment(Comment),
}

/// Whether optional blank lines are allowed in the current context.
/// The policy for [`Trivia::BlankLine`] line spacing.
///
/// Blank lines before comments and between comments are always permitted.
#[derive(Eq, PartialEq, Default, Debug, Clone, Copy)]
pub enum LineSpacingPolicy {
/// Blank lines are allowed before comments.
BeforeComments,
pub enum TriviaBlankLineSpacingPolicy {
/// Blank lines are allowed before and between comments, but not after.
///
/// i.e. a comment, then a blank line, then code, would have the blank
/// removed.
RemoveTrailingBlanks,
/// Blank lines are always allowed.
#[default]
Always,
Expand Down
Loading
Loading