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

Formatting if expressions can have problems when applying the short syntax style. #21530

Closed
nojaf opened this issue Nov 12, 2020 · 1 comment · Fixed by #21576
Closed

Formatting if expressions can have problems when applying the short syntax style. #21530

nojaf opened this issue Nov 12, 2020 · 1 comment · Fixed by #21576

Comments

@nojaf
Copy link
Contributor

nojaf commented Nov 12, 2020

Hello,

I believe I found a weakness in the rule:

If either cond, e1 or e2 are longer, but not multi-line:

if cond
then e1
else e2

if e1 is also a SynExpr.IfThenElse it can lead to a compiler error.
Related Fantomas bug: fsprojects/fantomas#1243

                        if f.IsProperty then
                            if f.IsInstanceMember then
                                if f.IsDispatchSlot then 9 else 1
                            else 8
                        elif f.IsMember then
                            if f.IsInstanceMember then
                                if f.IsDispatchSlot then 11 else 2
                            else 10
                        else 3

was transformed to

                if f.IsProperty
                then if f.IsInstanceMember then if f.IsDispatchSlot then 9 else 1 else 8
                elif f.IsMember
                then if f.IsInstanceMember then if f.IsDispatchSlot then 11 else 2 else 10
                else 3

On a personal note, I never was overly fond of the longer, but not multi-line, it leaves a lot of room for interpretation what "longer" is.
My proposal would be to remove the rule and add a further restriction to If cond, e1 and e2 are short, simply write them on one line:, that e1 or e2 cannot be an if/then/else itself.


Document Details

Do not edit this section. It is required for docs.microsoft.com ➟ GitHub issue linking.

@cartermp
Copy link
Contributor

Thanks, that is good feedback. I'd be happy to accept a PR that adjusts the guidance!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants