Skip to content

Moving end bracket to same line for strict whitespace #1842

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

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

Moving end bracket to same line for strict whitespace #1842

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 8 additions & 0 deletions Src/CSharpier.Core/Xml/XNodePrinters/Element.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ Doc PrintLineAfterChildren()
return Doc.IfBreak(Doc.SoftLine, "", attrGroupId);
}

if (
rawNode.Nodes.LastOrDefault() is { } node
&& Tag.PrintParentClosingTagStartWithContent(node, context)
)
{
return Doc.Null;
}

if (
rawNode.Attributes.Length == 0
&& rawNode.Nodes is [{ NodeType: XmlNodeType.Text }]
Expand Down
78 changes: 35 additions & 43 deletions Src/CSharpier.Core/Xml/XNodePrinters/Tag.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ public static Doc PrintClosingTag(RawNode rawNode, PrintingContext context)
{
return Doc.Concat(
rawNode.IsEmpty ? Doc.Null : PrintClosingTagStart(rawNode, context),
PrintClosingTagEnd(rawNode, context)
rawNode.Nodes.LastOrDefault() is { } node
&& PrintParentClosingTagStartWithContent(node, context)
? Doc.Null
: Doc.Concat(
PrintClosingTagEndMarker(rawNode),
PrintClosingTagSuffix(rawNode, context)
)
);
}

Expand All @@ -63,14 +69,6 @@ public static Doc PrintClosingTagStartMarker(RawNode rawNode, PrintingContext co
return $"</{rawNode.Name}";
}

public static Doc PrintClosingTagEnd(RawNode rawNode, PrintingContext context)
{
return Doc.Concat(
PrintClosingTagEndMarker(rawNode),
PrintClosingTagSuffix(rawNode, context)
);
}

public static Doc PrintClosingTagEndMarker(RawNode rawNode)
{
return rawNode.IsEmpty ? "/>" : ">";
Expand All @@ -79,10 +77,22 @@ public static Doc PrintClosingTagEndMarker(RawNode rawNode)
public static Doc PrintClosingTagSuffix(RawNode rawNode, PrintingContext context)
{
return PrintParentClosingTagStartWithContent(rawNode, context)
? PrintClosingTagStartMarker(rawNode.Parent!, context)
// TODO #1842 review this https://github.com/belav/csharpier-repos/pull/167
// TODO #1842 this doesn't make any sense now
? PrintParentClosingTagStartWithContent(rawNode, context)
? Doc.Concat(
PrintClosingTagStartMarker(rawNode.Parent!, context),
PrintClosingTagEndMarker(rawNode.Parent!)
)
: PrintClosingTagStartMarker(rawNode.Parent!, context)
: NeedsToBorrowNextOpeningTagStartMarker(rawNode)
? PrintOpeningTagStartMarker(rawNode.NextNode!, context)
: Doc.Null;
: Doc.Null;
}

public static bool DoStuff(RawNode rawNode, PrintingContext context)
{
return PrintParentClosingTagStartWithContent(rawNode, context);
}

private static Doc PrintOpeningTagStartMarker(RawNode rawNode, PrintingContext context)
Expand All @@ -92,38 +102,23 @@ private static Doc PrintOpeningTagStartMarker(RawNode rawNode, PrintingContext c

private static bool NeedsToBorrowNextOpeningTagStartMarker(RawNode rawNode)
{
/*
* 123<p
* ^^
* >
*/
/* 123<p
^^
> */
return rawNode.NextNode is not null
&& !rawNode.NextNode.IsTextLike()
&& rawNode.IsTextLike()
&& rawNode.NodeType is XmlNodeType.Text and not XmlNodeType.CDATA
// && node.isTrailingSpaceSensitive
// prettier does something with removing end of line nodes and setting this value, I don't know
// that we have that functionality
// && !node.hasTrailingSpaces
;
&& rawNode.NodeType is XmlNodeType.Text and not XmlNodeType.CDATA;
}

private static bool PrintParentClosingTagStartWithContent(
public static bool PrintParentClosingTagStartWithContent(
RawNode rawNode,
PrintingContext context
)
{
/*
* <p>
* 123</p
* ^^^
* >
*
* 123</b
* ></a
* ^^^
* >
*/
/* <p>
123</p>
^^^^*/
// TODO #1790 we really want this last condition only if the indentation of the last line of the text value matches
// the indentation of the start element. Bleh.
/*
Expand Down Expand Up @@ -161,15 +156,12 @@ public static bool NeedsToBorrowParentOpeningTagEndMarker(
PrintingContext context
)
{
/*
* <p
* >123
* ^
*
* <p
* ><a
* ^
*/
/* <p
>123
^
<p
><a
^ */
return rawNode.XmlWhitespaceSensitivity is XmlWhitespaceSensitivity.Strict
&& rawNode.PreviousNode is null
&& rawNode.NodeType is XmlNodeType.Text
Expand Down
3 changes: 1 addition & 2 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/xml_strict/Conditions.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
Condition=" '$(MSBuildProjectName)' != 'Microsoft.TestCommon'
AND '$(MSBuildProjectName)' != 'System.Net.Http.Formatting.NetCore.Test'
AND '$(MSBuildProjectName)' != 'System.Net.Http.Formatting.NetStandard.Test' "
>v4.5.2</TargetFrameworkVersion
>
>v4.5.2</TargetFrameworkVersion>
</PropertyGroup>
</Project>
6 changes: 4 additions & 2 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/xml_strict/Elements.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
<ElementWithAttribute Attribute="AttributeValue________________">TextValue</ElementWithAttribute>
<ElementWithAttribute Attribute="AttributeValue_________________">TextValue</ElementWithAttribute>
<ElementWithAttribute Attribute="AttributeValue__________________"
>TextValue</ElementWithAttribute
>
>TextValue</ElementWithAttribute>
<LongElementWithAttribute
Attribute="AttributeValue_________________________________________________________"
>TextValue</LongElementWithAttribute>
<ElementWithEncodedText>&lt;b&gt;SomeText&lt;/b&gt;</ElementWithEncodedText>
</Root>
7 changes: 7 additions & 0 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/xml_strict/Elements_Blah.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
<PropertyGroup>
<IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
>true</IsTrimmable>
<IsAotCompatible
Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))"
>true</IsAotCompatible>
</PropertyGroup>
4 changes: 1 addition & 3 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/xml_strict/Elements_HtmlValues.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,5 @@
<HtmlTypeElement>Some text<a href="http://url.com">url.com</a> more text.</HtmlTypeElement>
<HtmlTypeElement
>Some long text to make things break <a href="http://url.com"
>url.com</a
> more text.</HtmlTypeElement
>
>url.com</a> more text.</HtmlTypeElement>
</Root>
9 changes: 3 additions & 6 deletions Src/CSharpier.Tests/FormattingTests/TestFiles/xml_strict/StrictWhitespace.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,13 @@
>A <see cref="T:System.Int32" /> containing a value that reflects the sort order of
<paramref name="x" /> as compared to <paramref name="y" />. The following table defines the conditions
under which the returned value is a negative number, zero, or a positive
number.</para
>
number.</para>
</returns>
<para>Some text that ends with a space. </para>
<para
>Some loooooooooooooooooooooooooooooooooong text with this <br /> and a space after this. </para
>
>Some loooooooooooooooooooooooooooooooooong text with this <br /> and a space after this. </para>
<exception cref="T:System.InvalidOperationException"
>The current instance is read-only and a set operation was attempted. </exception
>
>The current instance is read-only and a set operation was attempted. </exception>
<example>
<code>public void MethodName() {
}
Expand Down
Loading