Skip to content

Conversation

@dawedawe
Copy link
Member

@dawedawe dawedawe commented Jul 20, 2023

This makes use of the new modeling of SynMeasure.Divide.
Now that the measure1 is optional, we have a chance to see, if the original code had it or not.
fixes #2926 and #2927

- adjust to optional lhs of Measure.Divide
Copy link
Contributor

@nojaf nojaf left a comment

Choose a reason for hiding this comment

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

So far so good.
I think you can add some more tests where we now deal with trivia better.

| SynRationalConst.Integer i -> string i
| SynRationalConst.Rational(numerator, denominator, _) -> $"(%i{numerator}/%i{denominator})"
| SynRationalConst.Negate innerRc -> $"-{visit innerRc}"
| SynRationalConst.Integer(value = i) -> string i
Copy link
Contributor

Choose a reason for hiding this comment

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

If you have the ranges you want to use creationAide.TextFromSource where possible.

| Measure.Divide n ->
let lhs = n.LeftHandSide |> Option.map genMeasure |> Option.defaultValue id

lhs
Copy link
Contributor

Choose a reason for hiding this comment

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

Use optSingle genMeasure n.LeftHandSide

@dawedawe dawedawe requested a review from josh-degraw July 23, 2023 15:51
@dawedawe
Copy link
Member Author

Hey @josh-degraw ,
please review if you can spend some time on it.
Even though there is more to do regarding measures, I'd like to do a release for this.

@dawedawe dawedawe marked this pull request as ready for review July 23, 2023 16:17
@dawedawe dawedawe merged commit 08a5245 into fsprojects:main Jul 23, 2023
@dawedawe dawedawe linked an issue Jul 24, 2023 that may be closed by this pull request
4 tasks
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.

Block comments in measure are lost or restored twice and in wrong place left out lhs in SynMeasure.Divide should not be restored as SynMeasure.One

3 participants