implement new line for brace #92

po-miyasaka · 2020-03-21T15:19:04Z

resolved #49

As is

At first glance, it is difficult to understand the hierarchy

If the following is implimented,

struct A {
  let b: B
  let string: String
}

struct B {
  let string: String
  let int: Int
}

let target = A(b: B(string: "hoge", int: 0), string: "foo")
Debug.pp >>> target

the result will be the following.

A(b: B(string: "hoge",
       int: 0),
  string: "foo")

To be

Using indent and new-line make the hierarchy easier to understand.

the result will be the following.

A(
 b: B(
     string: "hoge",
     int: 0
    ),
 string: "foo"
)

YusukeHosonuma

Congrats for fist commit 🎉

YusukeHosonuma · 2020-03-22T02:43:58Z

Tests/SwiftPrettyPrintTests/Public/DebugTests.swift

+                         Dog(
+                            id: "pochi",
+                            name: "ポチ",
+                            nickname: nil,
+                            age: 3,
+                            homepage: https://www.google.com/
+                         )


I think about indent-size are should decided by Debug.Option.indent instead of class (or struct) name.

// indent = 2 Dog( id: "pochi", name: "ポチ", ... // indent = 4 Dog( id: "pochi", name: "ポチ", ...

What do you think?

Fixed like that the indent depends on Debug.Option.

YusukeHosonuma · 2020-03-22T02:50:31Z

Sources/Core/Formatter/MultilineFormatter.swift

@@ -66,18 +66,19 @@ class MultilineFormatter: PrettyFormatter {
    func objectString(


I though about this format are decided by new option item Option.debug. objectFormat in detail design of #49 😅
But it's not bad idea that support only this style...

Which is better for the user...?

What do you think?

I think option.objectFormat is not needed.

Sources/Core/Formatter/MultilineFormatter.swift

YusukeHosonuma · 2020-03-22T03:03:50Z

Tests/SwiftPrettyPrintTests/Public/DebugTests.swift

+                             "dog-1": Dog(
+                                         id: "pochi",
+                                         name: "ポチ",
+                                         nickname: nil,
+                                         age: 3,
+                                         homepage: https://www.google.com/
+                                      ),


Hmmm, should we change dictionary format too for consistency of format? (I didn't think about this)

"dog-1": Dog( id: "pochi", name: "ポチ", nickname: nil, age: 3, homepage: https://www.google.com/, // dictionary in dictionary dict: [ "apple": Apple( price: 100, sale: true ) , "orange": Orange( price: 100, sale: true ) ] ),

What do you think?

It's depends on MultilineFormatter.objectString.
I think this consistency is not bad.

Are you say in other words that this problem should fixed but not in this PR?

oh, it will be fixed this issue.
#95

codecov-io · 2020-03-22T06:01:55Z

Codecov Report

Merging #92 into master will increase coverage by 0.14%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #92      +/-   ##
==========================================
+ Coverage   77.25%   77.39%   +0.14%     
==========================================
  Files           9        9              
  Lines         321      323       +2     
==========================================
+ Hits          248      250       +2     
  Misses         73       73

Impacted Files	Coverage Δ
Sources/Core/Formatter/MultilineFormatter.swift	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1995ab4...62489dc. Read the comment docs.

Co-Authored-By: Yusuke Hosonuma <tobi462@gmail.com>

…eHosonuma/SwiftPrettyPrint into feature/new-line-break-style

YusukeHosonuma

Good work!
Please confirm last one comment from me.

YusukeHosonuma · 2020-03-22T08:12:26Z

Tests/SwiftPrettyPrintTests/Core/Formatter/MultilineFormatterTests.swift

+                 )
+        )
+        """
+
        formatter = MultilineFormatter(option: option(indent: 2))


Please add test when indent size are 4 too :)

added test!

YusukeHosonuma

Great!!

po-miyasaka · 2020-03-22T10:31:33Z

Thanks for reviewing!!!!

implement new line for brace

f423789

po-miyasaka requested a review from YusukeHosonuma March 21, 2020 15:19

miyashi added 2 commits March 22, 2020 01:45

multiline description for object

3e64bc5

fix test for multiline

fe4f34d

po-miyasaka force-pushed the feature/new-line-break-style branch from 582fad3 to fe4f34d Compare March 21, 2020 16:53

po-miyasaka marked this pull request as ready for review March 21, 2020 16:59

po-miyasaka added the feature: formatter label Mar 21, 2020

YusukeHosonuma reviewed Mar 22, 2020

View reviewed changes

miyashi added 2 commits March 22, 2020 14:59

set indent from debug-option

1eb5334

fix indent

ea3c41c

po-miyasaka and others added 3 commits March 22, 2020 15:04

use + for readable

b782ba1

Co-Authored-By: Yusuke Hosonuma <tobi462@gmail.com>

fix comment along new-style

f495cc6

Merge branch 'feature/new-line-break-style' of ssh://github.com/Yusuk…

407ca5a

…eHosonuma/SwiftPrettyPrint into feature/new-line-break-style

YusukeHosonuma approved these changes Mar 22, 2020

View reviewed changes

add test for 4 indent

62489dc

YusukeHosonuma approved these changes Mar 22, 2020

View reviewed changes

po-miyasaka merged commit 167281d into master Mar 22, 2020

po-miyasaka deleted the feature/new-line-break-style branch March 22, 2020 10:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

implement new line for brace #92

implement new line for brace #92

po-miyasaka commented Mar 21, 2020 •

edited

Loading

YusukeHosonuma left a comment

YusukeHosonuma Mar 22, 2020

po-miyasaka Mar 22, 2020 •

edited

Loading

YusukeHosonuma Mar 22, 2020

po-miyasaka Mar 22, 2020

YusukeHosonuma Mar 22, 2020

po-miyasaka Mar 22, 2020 •

edited

Loading

YusukeHosonuma Mar 22, 2020

po-miyasaka Mar 22, 2020

codecov-io commented Mar 22, 2020 •

edited

Loading

YusukeHosonuma left a comment

YusukeHosonuma Mar 22, 2020

po-miyasaka Mar 22, 2020

YusukeHosonuma left a comment

po-miyasaka commented Mar 22, 2020 •

edited

Loading

		@@ -66,18 +66,19 @@ class MultilineFormatter: PrettyFormatter {
		func objectString(

implement new line for brace #92

implement new line for brace #92

Conversation

po-miyasaka commented Mar 21, 2020 • edited Loading

As is

At first glance, it is difficult to understand the hierarchy

To be

Using indent and new-line make the hierarchy easier to understand.

YusukeHosonuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

po-miyasaka Mar 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

po-miyasaka Mar 22, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Mar 22, 2020 • edited Loading

Codecov Report

YusukeHosonuma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

YusukeHosonuma left a comment

Choose a reason for hiding this comment

po-miyasaka commented Mar 22, 2020 • edited Loading

po-miyasaka commented Mar 21, 2020 •

edited

Loading

po-miyasaka Mar 22, 2020 •

edited

Loading

po-miyasaka Mar 22, 2020 •

edited

Loading

codecov-io commented Mar 22, 2020 •

edited

Loading

po-miyasaka commented Mar 22, 2020 •

edited

Loading