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

Correctly work formatting double splat argument with trailing comma #4990

Conversation

makenowjust
Copy link
Contributor

Below code is correct Crystal program, but crystal tool format cannot format it.

def foo(**x,)
end
$ crystal tool format foo.cr
Error:, couldn't format 'foo.cr', please report a bug including the contents of it: https://github.com/crystal-lang/crystal/issues

makenowjust added a commit to makenowjust/crystal that referenced this pull request Sep 17, 2017
Fix crystal-lang#1924
Close crystal-lang#4980, crystal-lang#4990

This is just refactoring but it contains many bug fixes and improvements.
These come from refactoring effects, so I cannot separate them from this.

Bug fixes and improvements:

  - work formatting double splat with trailing comma (crystal-lang#4990)
  - work formatting when there is newline or comment between argument and comma.

    ```crystal
    def foo(a
            ,)
    end

    def foo(a # comment
            ,)
    end
    ```

    is formatted to correctly:

    ```crystal
    def foo(a)
    end

    def foo(a # comment
            )
    end
    ```
  - format arguments with 2 spaces indentation (crystal-lang#1924)
  - distinguish comment followed on newline and argument.
    The formatter keeps this for example:

    ```crystal
    def foo(a # argument 'a' is ...
            )
    end

    def foo(a
            # more argument here in future...
            )
    end
    ```

And, perhaps there are other bug fixes and improvements because
`format_def_args` has too many bugs to remember.

Refactoring is so great!!
mverzilli pushed a commit that referenced this pull request Sep 17, 2017
Fix #1924
Close #4980, #4990

This is just refactoring but it contains many bug fixes and improvements.
These come from refactoring effects, so I cannot separate them from this.

Bug fixes and improvements:

  - work formatting double splat with trailing comma (#4990)
  - work formatting when there is newline or comment between argument and comma.

    ```crystal
    def foo(a
            ,)
    end

    def foo(a # comment
            ,)
    end
    ```

    is formatted to correctly:

    ```crystal
    def foo(a)
    end

    def foo(a # comment
            )
    end
    ```
  - format arguments with 2 spaces indentation (#1924)
  - distinguish comment followed on newline and argument.
    The formatter keeps this for example:

    ```crystal
    def foo(a # argument 'a' is ...
            )
    end

    def foo(a
            # more argument here in future...
            )
    end
    ```

And, perhaps there are other bug fixes and improvements because
`format_def_args` has too many bugs to remember.

Refactoring is so great!!

* Rename 'wrote_newline' inside 'format_def_arg' to 'just_wrote_newline'

And fix some conditions for readbility.
@miketheman
Copy link
Contributor

@makenowjust Is this now solved also by #4992 ? I think I saw similar double-splat test cases included in that one.

@miketheman
Copy link
Contributor

Oddly, GitHub did not auto-close this Pull Request.

@miketheman
Copy link
Contributor

Looks like you would have to repeat the Close #NNN syntax for each one to act on:

screenshot 2017-09-17 10 26 15

@asterite
Copy link
Member

Closed by #4992

@asterite asterite closed this Sep 17, 2017
@makenowjust makenowjust deleted the fix/crystal-format/double-splat-trailing-comma branch September 27, 2017 10:04
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.

None yet

3 participants