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.

Sign up for GitHub

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

Avoid multiline opening tag for single attribute #42

Merged
merged 8 commits into from
Feb 12, 2025
Merged

Avoid multiline opening tag for single attribute #42

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7022cfd
Add test cases single attr per html element
UnknownPlatypus Jul 23, 2024
d7532a1
Prevent wrapping for single attributes
UnknownPlatypus Feb 3, 2025
81ceae9
[Refacto] Compute `formatted_tag_name` only once
UnknownPlatypus Aug 9, 2024
816e955
Add jinja tests and ignore jinja blocks spanning multiple lines
UnknownPlatypus Feb 3, 2025
e3e50c2
Keep on multiple lines if the attribute contains a newline
UnknownPlatypus Feb 8, 2025
78d8253
Trim before cheking for newlines for native attributes
UnknownPlatypus Feb 9, 2025
f7d4eaa
feat: optimize quotes of attr value in Jinja (#102)
UnknownPlatypus Feb 12, 2025
c985ade
accept code review
g-plane Feb 12, 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
4 changes: 1 addition & 3 deletions dprint_plugin/tests/integration/biome/quotes.vue.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,7 @@ source: dprint_plugin/tests/integration.rs
<div v-if="(label = 'a')"></div>
<div v-if="(label = 'a')"></div>

<button
@click="(content += '{&quot;hello&quot;: &quot;I\'m a button!&quot;}')"
>
<button @click="(content += '{&quot;hello&quot;: &quot;I\'m a button!&quot;}')">
</button>

<input :value="''">
Expand Down
4 changes: 1 addition & 3 deletions dprint_plugin/tests/integration/biome/style-attr.html.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,5 @@ source: dprint_plugin/tests/integration.rs
---
<div style="width: 1px; height: 1px"></div>

<ul
style="display: grid; grid-template-columns: 50% 50%; justify-items: stretch; padding: 0"
>
<ul style="display: grid; grid-template-columns: 50% 50%; justify-items: stretch; padding: 0">
</ul>
4 changes: 1 addition & 3 deletions dprint_plugin/tests/integration/dprint_ts/style-attr.html.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,5 @@ source: dprint_plugin/tests/integration.rs
---
<div style="width: 1px; height: 1px"></div>

<ul
style="display: grid; grid-template-columns: 50% 50%; justify-items: stretch; padding: 0"
>
<ul style="display: grid; grid-template-columns: 50% 50%; justify-items: stretch; padding: 0">
</ul>
317 changes: 183 additions & 134 deletions markup_fmt/src/printer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,19 +400,24 @@ impl<'s> DocGen<'s> for Element<'s> {
.split_once(':')
.and_then(|(namespace, name)| namespace.eq_ignore_ascii_case("html").then_some(name))
.unwrap_or(self.tag_name);
let formatted_tag_name = if matches!(
ctx.language,
Language::Html | Language::Jinja | Language::Vento
) && css_dataset::tags::STANDARD_HTML_TAGS
.iter()
.any(|tag| tag.eq_ignore_ascii_case(self.tag_name))
{
Cow::from(self.tag_name.to_ascii_lowercase())
} else {
Cow::from(self.tag_name)
};
let is_root = state.is_root;
let mut state = State {
current_tag_name: Some(tag_name),
is_root: false,
in_svg: tag_name.eq_ignore_ascii_case("svg"),
indent_level: state.indent_level,
};
let should_lower_cased = matches!(
ctx.language,
Language::Html | Language::Jinja | Language::Vento
) && css_dataset::tags::STANDARD_HTML_TAGS
.iter()
.any(|tag| tag.eq_ignore_ascii_case(self.tag_name));

let self_closing = if helpers::is_void_element(tag_name, ctx.language) {
ctx.options
Expand Down Expand Up @@ -447,115 +452,128 @@ impl<'s> DocGen<'s> for Element<'s> {
let mut docs = Vec::with_capacity(5);

docs.push(Doc::text("<"));
docs.push(Doc::text(if should_lower_cased {
Cow::from(self.tag_name.to_ascii_lowercase())
} else {
Cow::from(self.tag_name)
}));

let attrs_sep = if !self.first_attr_same_line
&& !ctx.options.prefer_attrs_single_line
&& self.attrs.len() > 1
&& !ctx
.options
.max_attrs_per_line
.map(|value| value.get() > 1)
.unwrap_or_default()
{
Doc::hard_line()
} else {
Doc::line_or_space()
};
let attrs = if let Some(max) = ctx.options.max_attrs_per_line {
// fix #2
if self.attrs.is_empty() {
Doc::line_or_nil()
} else {
Doc::line_or_space()
docs.push(Doc::text(formatted_tag_name.clone()));

match self.attrs.as_slice() {
[attr] if !is_whitespace_sensitive && !is_multi_line_attr(attr) => {
docs.push(Doc::space());
docs.push(attr.doc(ctx, &state));
if self_closing && is_empty {
docs.push(Doc::text(" />"));
return Doc::list(docs);
} else {
docs.push(Doc::text(">"));
};
if self.void_element {
return Doc::list(docs);
}
}
.concat(itertools::intersperse(
self.attrs.chunks(max.into()).map(|chunk| {
_ => {
let attrs_sep = if !self.first_attr_same_line
&& !ctx.options.prefer_attrs_single_line
&& self.attrs.len() > 1
&& !ctx
.options
.max_attrs_per_line
.map(|value| value.get() > 1)
.unwrap_or_default()
{
Doc::hard_line()
} else {
Doc::line_or_space()
};
let attrs = if let Some(max) = ctx.options.max_attrs_per_line {
// fix #2
if self.attrs.is_empty() {
Doc::line_or_nil()
} else {
Doc::line_or_space()
}
.concat(itertools::intersperse(
self.attrs.chunks(max.into()).map(|chunk| {
Doc::list(
itertools::intersperse(
chunk.iter().map(|attr| attr.doc(ctx, &state)),
attrs_sep.clone(),
)
.collect(),
)
.group()
}),
Doc::hard_line(),
))
.nest(ctx.indent_width)
} else {
Doc::list(
itertools::intersperse(
chunk.iter().map(|attr| attr.doc(ctx, &state)),
attrs_sep.clone(),
)
.collect(),
self.attrs
.iter()
.flat_map(|attr| [attrs_sep.clone(), attr.doc(ctx, &state)].into_iter())
.collect(),
)
.group()
}),
Doc::hard_line(),
))
.nest(ctx.indent_width)
} else {
Doc::list(
self.attrs
.iter()
.flat_map(|attr| [attrs_sep.clone(), attr.doc(ctx, &state)].into_iter())
.collect(),
)
.nest(ctx.indent_width)
};
.nest(ctx.indent_width)
};

if self.void_element {
docs.push(attrs);
if self_closing {
docs.push(Doc::line_or_space());
docs.push(Doc::text("/>"));
} else {
if !ctx.options.closing_bracket_same_line {
docs.push(Doc::line_or_nil());
}
docs.push(Doc::text(">"));
}
return Doc::list(docs).group();
}
if self_closing && is_empty {
docs.push(attrs);
docs.push(Doc::line_or_space());
docs.push(Doc::text("/>"));
return Doc::list(docs).group();
}
if ctx.options.closing_bracket_same_line {
docs.push(attrs.append(Doc::text(">")).group());
} else {
// for #16
if is_whitespace_sensitive
&& !self.attrs.is_empty() // there're no attributes, so don't insert line break
&& self
.children
.first()
.is_some_and(|child| {
if let NodeKind::Text(text_node) = &child.kind {
!text_node.raw.starts_with(|c: char| c.is_ascii_whitespace())
} else {
false
}
})
&& self
.children
.last()
.is_some_and(|child| {
if let NodeKind::Text(text_node) = &child.kind {
!text_node.raw.ends_with(|c: char| c.is_ascii_whitespace())
} else {
false
if self.void_element {
docs.push(attrs);
if self_closing {
docs.push(Doc::line_or_space());
docs.push(Doc::text("/>"));
} else {
if !ctx.options.closing_bracket_same_line {
docs.push(Doc::line_or_nil());
}
})
{
docs.push(
attrs
.group()
.append(Doc::line_or_nil())
.append(Doc::text(">")),
);
} else {
docs.push(
attrs
.append(Doc::line_or_nil())
.append(Doc::text(">"))
.group(),
);
docs.push(Doc::text(">"));
}
return Doc::list(docs).group();
}
if self_closing && is_empty {
docs.push(attrs);
docs.push(Doc::line_or_space());
docs.push(Doc::text("/>"));
return Doc::list(docs).group();
}
if ctx.options.closing_bracket_same_line {
docs.push(attrs.append(Doc::text(">")).group());
} else {
// for #16
if is_whitespace_sensitive
&& !self.attrs.is_empty() // there're no attributes, so don't insert line break
&& self
.children
.first()
.is_some_and(|child| {
if let NodeKind::Text(text_node) = &child.kind {
!text_node.raw.starts_with(|c: char| c.is_ascii_whitespace())
} else {
false
}
})
&& self
.children
.last()
.is_some_and(|child| {
if let NodeKind::Text(text_node) = &child.kind {
!text_node.raw.ends_with(|c: char| c.is_ascii_whitespace())
} else {
false
}
})
{
docs.push(
attrs
.group()
.append(Doc::line_or_nil())
.append(Doc::text(">")),
);
} else {
docs.push(
attrs
.append(Doc::line_or_nil())
.append(Doc::text(">"))
.group(),
);
}
}
}
}

Expand Down Expand Up @@ -756,11 +774,7 @@ impl<'s> DocGen<'s> for Element<'s> {

docs.push(
Doc::text("</")
.append(Doc::text(if should_lower_cased {
Cow::from(self.tag_name.to_ascii_lowercase())
} else {
Cow::from(self.tag_name)
}))
.append(Doc::text(formatted_tag_name))
.append(Doc::line_or_nil())
.append(Doc::text(">"))
.group(),
Expand Down Expand Up @@ -963,25 +977,7 @@ impl<'s> DocGen<'s> for NativeAttribute<'s> {
}
_ => Cow::from(value),
};
let has_single = value.contains('\'');
let has_double = value.contains('"');
let quote = if has_double && has_single {
if let Some(quote) = self.quote {
Doc::text(quote.to_string())
} else if let Quotes::Double = ctx.options.quotes {
Doc::text("\"")
} else {
Doc::text("'")
}
} else if has_double {
Doc::text("'")
} else if has_single {
Doc::text("\"")
} else if let Quotes::Double = ctx.options.quotes {
Doc::text("\"")
} else {
Doc::text("'")
};
let quote = compute_attr_value_quote(&value, self.quote, ctx);
let mut docs = Vec::with_capacity(5);
docs.push(name);
docs.push(Doc::text("="));
Expand Down Expand Up @@ -1984,6 +1980,25 @@ fn is_all_ascii_whitespace(s: &str) -> bool {
!s.is_empty() && s.as_bytes().iter().all(|byte| byte.is_ascii_whitespace())
}

fn is_multi_line_attr(attr: &Attribute) -> bool {
match attr {
Attribute::Native(attr) => attr
.value
.map(|(value, _)| value.trim().contains('\n'))
.unwrap_or(false),
Attribute::VueDirective(attr) => attr
.value
.map(|(value, _)| value.contains('\n'))
.unwrap_or(false),
Attribute::Astro(attr) => attr.expr.0.contains('\n'),
Attribute::Svelte(attr) => attr.expr.0.contains('\n'),
Attribute::JinjaComment(comment) => comment.raw.contains('\n'),
Attribute::JinjaTag(tag) => tag.content.contains('\n'),
// Templating blocks usually span across multiple lines so let's just assume true.
Attribute::JinjaBlock(..) | Attribute::VentoTagOrBlock(..) => true,
}
}

fn should_ignore_node<'s, E, F>(index: usize, nodes: &[Node], ctx: &Ctx<'s, E, F>) -> bool
where
F: for<'a> FnMut(&'a str, Hints) -> Result<Cow<'a, str>, E>,
Expand Down Expand Up @@ -2414,3 +2429,37 @@ where
state,
)))
}

/// Computes the appropriate quote character (single or double) to use for an attribute value.
///
/// This will try to respect the configured `quotes` but might change it
/// to ensure the result is valid html.
fn compute_attr_value_quote<'s, E, F>(
attr_value: &str,
initial_quote: Option<char>,
ctx: &mut Ctx<'s, E, F>,
) -> Doc<'s>
where
F: for<'a> FnMut(&'a str, Hints) -> Result<Cow<'a, str>, E>,
{
let is_jinja = matches!(ctx.language, Language::Jinja);
let has_single = attr_value.contains('\'');
let has_double = attr_value.contains('"');
if has_double && has_single {
if let Some(quote) = initial_quote {
Doc::text(quote.to_string())
} else if let Quotes::Double = ctx.options.quotes {
Doc::text("\"")
} else {
Doc::text("'")
}
} else if has_double && !is_jinja {
Doc::text("'")
} else if has_single && !is_jinja {
Doc::text("\"")
} else if let Quotes::Double = ctx.options.quotes {
Doc::text("\"")
} else {
Doc::text("'")
}
}
Loading