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

Fix various issues with Extensions.csproj handling and add unit testing for it #2180

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

Conversation

PathogenDavid
Copy link
Member

@PathogenDavid PathogenDavid commented Mar 11, 2025

Was initially focused on #2055, which was caused by an oversight done when refactoring the several redundant XML parsing entrypoints during #1753 (Prior to the refactor, all of them stripped whitespace except for the one that loaded the project for adding package references. That exception was lost in the refactor.)

While I was at it I wrote a suite of unit tests related to the handling of Extensions.csproj and fixed some other issues I found along the way.

  • Fix whitespace being stripped during automatic updates
  • Fix fresh Extensions.csproj always being written with CRLF newlines
  • Fix ScriptExtensionsProjectMetadata not actually being immutable as intended
  • Fix adding redundant ItemGroup when all PackageReferences were removed but the group wasn't
  • Added automatic exposing of assembly internals to the relevant test assembly
  • Upgraded to C# 12 to get raw strings (our global.json already requires the .NET 8 SDK anyway)

Fixes #2055

…ting for it

* Fix whitespace being stripped during automatic updates
* Fix fresh `Extensions.csproj` always being written with CRLF newlines
* Fix `ScriptExtensionsProjectMetadata` not actually being immutable as intended
* Fix adding redundant `ItemGroup` when all `PackageReference`s were removed but the group wasn't
* Added automatic exposing of assembly internals to the relevant test assembly
* Upgraded to C# 12 to get raw strings (our `global.json` already requires the .NET 8 SDK anyway)

Fixes bonsai-rx#2055
@PathogenDavid PathogenDavid requested a review from glopesdev March 11, 2025 01:38
CollectionAssert.AreEquivalent((string[])["FakePackageA", "FakePackageB"], metadata.GetPackageReferences().ToArray());

// Package was not updated
//TODO: This is how the implementation is written, but is this behavior correct?
Copy link
Member Author

Choose a reason for hiding this comment

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

This test covers the behavior provided by this check:

else if (NuGetVersion.Parse(version.Value) < NuGetVersion.Parse(reference.Version))

It's been like this since 2018 when automatic Extensions.csproj updating was introduced.

Is this behavior actually a good idea? It seems to me like the Extensions.csproj should always match the Bonsai environment since the two not matching only causes confusion.

@PathogenDavid
Copy link
Member Author

Do we want to special-case saving to automatically re-add newlines if the original project had none in order to undo the damage caused by #2055 ?

Should be as simple as skipping SaveOptions.DisableFormatting if the first descendant of the Root is an XText not containing a \n, but on the other hand anyone who cares can just ask their IDE to reformat the file to restore sensible whitespace.

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.

Extensions cproj is being formated by the editor and stripped of NL
1 participant