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

Update ScintillaNET to support modern .NET #1924

Merged
merged 2 commits into from
Jul 21, 2024

Conversation

glopesdev
Copy link
Member

@glopesdev glopesdev commented Jul 16, 2024

In preparation for modernizing the editor to run with the .NET Core stack there are a number of core dependencies which need to be upgraded or modified to run beyond .NET Framework. jacobslusser/ScintillaNET was until now one of the big concerns, since the project has been stale for almost 6+ years.

Fortunately, fernandreu/ScintillaNET now provides a .NET 6.0 compatible fork with a published NuGet package and the source code updates seem to be in great shape. Even though it still only targets windows, the component has even been updated with a new arm64 runtime, which is a welcome addition for possibly targeting Windows on ARM.

Since this fork only targets .NET 4.7.1 we took the chance to update all projects to target .NET 4.7.2 at a minimum, which is now our recommended target for new packages anyway.

@glopesdev glopesdev added feature New planned feature fix Pull request that fixes an issue labels Jul 16, 2024
@glopesdev glopesdev added this to the 2.9 milestone Jul 16, 2024
@glopesdev glopesdev requested a review from PathogenDavid July 16, 2024 13:47
@PathogenDavid PathogenDavid changed the title Update ScintillaNET to support netcore Update ScintillaNET to support modern .NET Jul 19, 2024
Copy link
Member

@PathogenDavid PathogenDavid left a comment

Choose a reason for hiding this comment

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

Does this PR actually fix #1706? That issue seems to be primarily about moving this dependency out of Bonsai.Design, which is not what's being done here. (The comment is about that, but the comment also seems mostly unrelated to the main issue?)

Other than that the PR looks good.

@glopesdev
Copy link
Member Author

Good point, I have amended the wording to "closes".

@glopesdev glopesdev merged commit 69f4267 into bonsai-rx:main Jul 21, 2024
10 checks passed
@glopesdev glopesdev deleted the issue-1706 branch July 21, 2024 22:03
@PathogenDavid
Copy link
Member

@glopesdev To clarify: I don't see how this PR addresses #1706 at all. Fixing closing or otherwise. It addresses the comment, but it's also not clear how that comment really relates to the actual issue 😅

Migrating to the new package doesn't really solve the issue of imposing this package on downstream consumers of Bonsai.Design. Was #1706 intended primarily as paving a path forward for modern .NET support rather than fixing the unnecessary transitive dependency?

@glopesdev
Copy link
Member Author

@PathogenDavid agreed, I was distracted by another shared dependency RichTextEditor (long story) but it ends up not relating to this upstream dependency at all, the issue still stands, will reopen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New planned feature fix Pull request that fixes an issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants