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

Refactor 'Crystal::Formatter#format_def_args' #4992

Conversation

makenowjust
Copy link
Contributor

@makenowjust makenowjust commented 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:

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

Refactoring is so great!!


I apologize to @asterite and other members for big commit. But I'm sure this is the best solution because fundamental problem is format_def_args is so complex. If we resolved issues explained in above separately, then format_def_args would becomes more complex and it means refactoring becomes harder. I don't hope such a thing.

Please read the source code. I believe it is clearer and simpler then past.

Thank you.

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!!
@makenowjust makenowjust force-pushed the feature/crystal-format/refactor-format-def-args branch from eb4fc13 to 58ea85c Compare September 17, 2017 11:38
@asterite
Copy link
Member

@makenowjust This is excellent! No need to apologize at all! :-)

The code looks much better this way. Thank you!

@mverzilli mverzilli merged commit 74050a4 into crystal-lang:master Sep 17, 2017
@makenowjust makenowjust deleted the feature/crystal-format/refactor-format-def-args branch September 19, 2017 03:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Formatter indentation not matching with what I'd expect
4 participants