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

Review potential APIs to help migration to Farkle 7 and decide on them. #351

Open
teo-tsirpanis opened this issue Jan 4, 2025 · 0 comments · May be fixed by #366
Open

Review potential APIs to help migration to Farkle 7 and decide on them. #351

teo-tsirpanis opened this issue Jan 4, 2025 · 0 comments · May be fixed by #366

Comments

@teo-tsirpanis
Copy link
Owner

teo-tsirpanis commented Jan 4, 2025

As we are getting closer to Farkle 7 Preview 1, we want to provide a very smooth migration experience. After taking a look at the projects that use Farkle, there are uses of the some APIs that do not exist in Farkle 7 at the moment, and we have to make a decision on whether to add them (deprecated or not) or make a breaking change.

The objective is that projects that upgrade to Farkle 7 encounter as few compile errors as possible. This is easier to do in F# because it supports type aliases, and any new APIs added for compatibility can be more easily removed since they won't cause binary breaking changes.

Will add

  • C# overloads to Regex.(Not)OneOf that accept an IEnumerable<char>.
    • Easy to add, and passing a string is a valid scenario.
  • Regex.Or and And instance methods.
    • Easy to add, will deprecate and hide. The | and + operators are much cleaner, and the name of And for regex concatenation is unfortunate.
  • MarkForPrecompile (C#/F#) and buildPrecompiled (F#).
    • Will add with dummy implementations (identity function and call build respectively) and hide, but not deprecate. The precompiler will not land in Preview 1, but we want to keep usage of these APIs around, so that once it lands, we then deprecate them, and provide guidance on how to migrate to the new precompiler API.

Needs more thought Will add

  • Terminal.Create overloads with previous parameter order.
    • In Farkle 6, the Terminal.Create function takes the name, the transformer and the regex in this order, but in Farkle 7 the transformer was moved to the end, which is cleaner because the transformer might be a multi-line lambda. We have to consider any implications around overload resolution before we decide whether to add an overload with the previous order (will hide but it's too minor to deprecate), or bite the bullet.
      Update: Will add as non-obsolete, hidden, and also with [OverloadResolutionPriority(-1)] out of abundance of caution.
  • DesigntimeFarkle, RuntimeFarkle and friends for C#.
    • Farkle 7 renamed its foundational parser and builder types. In F# they are available under their previous name thanks to some deprecated type aliases, but since C# does not have type aliases, trying to emulate them would be very challenging, compared to passing the burden of migration to users (which is mostly a find-replace operation).
      But on second thought, we could package a C# source file with dummy definitions of these types, that will convert the "type not found" error into an "obsolete, here's how to fix it" warning. We can remove this file in a post-Farkle 7.0 release while maintaining binary compatibility.
      Update: Will add as obsolete-as-error and hidden. No functionality will be added to these types and nor will they be assignable from the new parser and builder types; they will exist solely to provide a nice error message. Adding it in a C# source file will not have any difference over in the binary; we won't care about binary compatibility when we remove it.

Will not add

  • PredefinedSets
    • After more thought, the verdict on the fate of PredefinedSets remains the same. It has some but few uses that are all easy to migrate, and is a feature that is not desired to continue supporting for Farkle 7.
  • Terminal.Create overloads with stateless transformer.
    • This is a potential new API. For some context, in Farkle 7, the ITransformerContext type was renamed to ParserState and became a struct that gets passed by reference. Unfortunately this means that when declaring the transformer as a lambda in C#1, (_, data) => has to become (ref ParserState _, ReadOnlySpan<char> data) =>. In C# 14 the simple lambda parameters language feature will reduce this to (ref _, data) => which is acceptable, but until it is shipped, user code will be more verbose.
      There was the idea of introducing a second kind of transformer delegates that accepts only a span (and overloads to Terminal.Create respectively), but considering that simple lambda parameters has been merged, a workaround in Farkle's side will have no long-term benefits, and supporting two kinds of transformers might introduce some other complications in the future.

Footnotes

  1. This change is completely transparent for F# users that rely on the language's type inference.

@teo-tsirpanis teo-tsirpanis linked a pull request Feb 24, 2025 that will close this issue
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 a pull request may close this issue.

1 participant