Questionable formatting in `xml`/`csproj` files

[Tyrrrz]

I generally like the fact that Csharpier picks up XML files now, however some formatting rules are a bit questionable.

Input:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net9.0</TargetFrameworks>
    <IsPackable>true</IsPackable>
    <IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable>
    <IsAotCompatible Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible>
  </PropertyGroup>

Output:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net9.0</TargetFrameworks>
    <IsPackable>true</IsPackable>
    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
      >true</IsTrimmable
    >
    <IsAotCompatible
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))"
      >true</IsAotCompatible
    >
  </PropertyGroup>

Expected behavior:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net9.0</TargetFrameworks>
    <IsPackable>true</IsPackable>
    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
      >true</IsTrimmable>
    <IsAotCompatible
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))"
      >true</IsAotCompatible>
  </PropertyGroup>

Or, probably better:

  <PropertyGroup>
    <TargetFrameworks>netstandard2.0;net9.0</TargetFrameworks>
    <IsPackable>true</IsPackable>
    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable>
    <IsAotCompatible
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net7.0'))">true</IsAotCompatible>
  </PropertyGroup>

Additionally, why does CSharpier remove empty newlines in XML? I think newlines are useful to separate certain element blocks, like <ItemGroup>.

[asbjornb]

Worst has been for our <NoWarn> sections that formats existing:

    <NoWarn>
      CA1031; <!-- Since this is not a library project, catching general exceptions is OK -->
      IDE0005; <!-- Allow unused usings -->
    </NoWarn>

To:

    <NoWarn
      >
      CA1031;

      <!-- Since this is not a library project, catching general exceptions is OK -->

      
      IDE0005;

      <!-- Allow unused usings -->
    </NoWarn>

[belav]

The xml formatting is a port of prettier's html formatting which is how it ended up this way. I've gotten used to it.

    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
      >true</IsTrimmable
    >

I don't know that I think this is better, because the true value doesn't appear until the end of the line

    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">true</IsTrimmable>

I'm not opposed to this. Prettier has it as an option but it is not the default.

    <IsTrimmable
      Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
      >true</IsTrimmable>

Another possibility would be treating whitespace as not strict, although that could cause issues if the file being formatted does have strict whitespace.

<IsTrimmable Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">
  true
</IsTrimmable>

<!-- except if you have many attributes you don't want them to align with the content -->
<IsTrimmable
  Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
  Condition2="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))">
  true
</IsTrimmable>

<!-- vs -->
<IsTrimmable
  Condition="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
  Condition2="$([MSBuild]::IsTargetFrameworkCompatible('$(TargetFramework)', 'net6.0'))"
>
  true
</IsTrimmable>

[belav]

For respecting empty lines - #1599

@asbjornb what you are seeing looks like a bug, I created #1607

[Tyrrrz]

Yeah, unfortunately many MSBuild properties are sensitive to the surrounding whitespace, so can't treat it as non-strict :(

[belav]

Resolved in #1842