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

Long type argument with constraints is not respecting max_line_length #2266

Closed
3 tasks
nojaf opened this issue May 18, 2022 · 9 comments
Closed
3 tasks

Long type argument with constraints is not respecting max_line_length #2266

nojaf opened this issue May 18, 2022 · 9 comments

Comments

@nojaf
Copy link
Contributor

nojaf commented May 18, 2022

Issue created from fantomas-online

Code

type Event<'Delegate, 'Args when 'Delegate: delegate<'Args, unit> and 'Delegate :> System.Delegate and 'Delegate: not struct> () =
                        class end

Result

type Event<'Delegate, 'Args when 'Delegate: delegate<'Args, unit> and 'Delegate :> System.Delegate and 'Delegate: not struct>
    () =
    class
    end

Problem description

Reading https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-explicit-generic-type-arguments-and-constraints and https://docs.microsoft.com/en-us/dotnet/fsharp/style-guide/formatting#formatting-function-and-member-arguments, I think this should be more like:

type Event<'Delegate, 'Args 
    when 'Delegate: delegate<'Args, unit> 
    and 'Delegate :> System.Delegate 
    and 'Delegate: not struct>
    () 
    =
    class
    end

Extra information

  • The formatted result breaks my code.
  • The formatted result gives compiler warnings.
  • I or my company would be willing to help fix this.

Options

Fantomas master branch at 2022-05-18T11:51:41Z - ce92e66

Default Fantomas configuration

Did you know that you can ignore files when formatting from fantomas-tool or the FAKE targets by using a .fantomasignore file?

@nojaf
Copy link
Contributor Author

nojaf commented May 18, 2022

@dsyme should we introduce a label to annotate all problems related to formatting dotnet/fsharp?

@dsyme
Copy link
Contributor

dsyme commented May 18, 2022

@dsyme should we introduce a label to annotate all problems related to formatting dotnet/fsharp?

No need I think - it's not much difference from the fantomas perspective, and at this stage nothing is blocked in dotnet/fsharp

@dsyme
Copy link
Contributor

dsyme commented May 18, 2022

I'm not too sure about the style guide here, it does feel like it's harder to read with the constraints only indented so far. However I'm not sure what to do about it, c.f.

type Event<'Delegate, 'Args 
                when 'Delegate: delegate<'Args, unit> 
                and 'Delegate :> System.Delegate 
                and 'Delegate: not struct>
    () 
    =
    class
    end

@nojaf
Copy link
Contributor Author

nojaf commented May 19, 2022

But Don, that would be a vanity alignment.
Jokes aside, I don't see any alternative.
I think you would get used to this over time.

@dsyme
Copy link
Contributor

dsyme commented May 19, 2022

Perhaps multi line type args should go a bit like multi line term args, eg

type Event<
    'Delegate,
    'Args 
    when
        'Delegate: delegate<'Args, unit> and
        'Delegate :> System.Delegate and
        'Delegate: not struct
> () 
    =
    class
    end

@dsyme
Copy link
Contributor

dsyme commented May 19, 2022

4-space or 8-space. I guess the latter is vanity and the former is not?

@nojaf
Copy link
Contributor Author

nojaf commented May 20, 2022

That isn't valid code I'm afraid, Incomplete structured construct at or before this point in type name. Expected '>' or other token.. (online tool).

How about this.
This feels a bit closer to what we had in the previous PR as well.

@knocte
Copy link
Contributor

knocte commented May 20, 2022

The < char being in the first line looks a bit funky, no?

How about:

type ALooooooooooooooooong
    <'TLooooooooooooong, 'VLooooooooooong 
        when 'TLooooooooooooong: delegate<'VLooooooooooong, unit> 
        and 'TLooooooooooooong :> System.Delegate
        and 'TLooooooooooooong: not struct> ()
    =
    class
    end

@nojaf
Copy link
Contributor Author

nojaf commented May 20, 2022

This again gives a warning...
Can everybody please verify the validity of their proposal before post in the future.
Thanks.

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

No branches or pull requests

3 participants