Update document to latest format #3621

kemchenj · 2018-09-04T13:51:23Z

Issue Link 🔗

Some of the document format were outdated and inconsistent. It leads to some weird compile warning like this:

⚠️  *********/Build/Products/Release-iphoneos/Charts/Charts.framework/Headers/
Charts-Swift.h:3208:12: parameter 'stackIndex' not found in the function declaration [-Wdocumentation]

/// \param stackIndex references which value of a stacked-bar entry has been selected

So I think is time to update the document format.

Goals ⚽

Update all the existing document to latest format.

Sort section title in order [Summary, Note, Parameters, Throws, Returns]

before:

/// Returns: A new value
/// Note: Some import thing
/// Throws: MoyaError
/// Parameters:
///   - foo: A Int Value
///   - bar: A Double Value

after:

/// Note: Some import thing
/// Parameters:
///   - foo: A Int value
///   - bar: A Double Value
/// Throws: MoyaError
/// Returns: A new value

Uppercased the section title
- /// - note: -> /// - Note:
- /// - parameter: -> /// - Parameters:
- /// - returns: -> /// - Returns:
- /// - throws: -> /// - Throws:

Update parameters section format

before:

/// parameter foo: A Int value
/// parameter bar: A Double value

after:

/// Parameters:
///   - foo: A Int value
///   - bar: A Double value

Move property's Returns section content to Summary

before:

/// - Returns: Some property
var property: Int

after:

/// Some property
var property: Int

Add empty line between summary and other section:

before:

/// Function description
/// Parameters:
///   - a: Some Int value
func foo(a: Int) { ... }

after:

/// Function description
///
/// Parameters:
///   - a: Some Int value
func foo(a: Int) { ... }

Implementation Details 🚧

Add missing -.
- Regex: /// (note|return|parameters|throws):
- Replacement: /// - $1:
Remove property's Returns section.
- Regex: /// - returns: (.+\s+((override|@IBOutlet|@objc|weak|unowned|lazy|static|class|open|public|private|fileprivate|internal)($set$)? )*(var|let))
- Replacement: /// $1

Update Parameters section format.

Add Parameters section.
- Regex: (\n[ ]+)(((/// - parameter \w+:.*\s+)(///((\s+)|( \s+.+\s+)))?)*/// - parameter \w+:.*)
- Replacement: $1/// - Parameters:$1$2
Remove all parameter prefix.
- Regex: /// - parameter (\w+):(.*)(\s+///(\n))*(\s+)
- Replacement: /// - $1:$2$4$5

Some edge case:

Charts/Source/Charts/Components/LegendEntry.swift

/// - parameter label:                  The legend entry text.
///                                     A `nil` label will start a group.
/// - parameter form:                   The form to draw for this entry.

Charts/Source/Charts/Formatters/IValueFormatter.swift

/// - parameter value:           The value to be formatted
///
/// - parameter dataSetIndex:    The index of the DataSet the entry in focus belongs to
///
/// - parameter viewPortHandler: provides information about the current chart state (scale, translation, ...)

Sort sections and add empty line between Summary and other sections.
- Regex: ((///)[ ]+[^-\n]+(\s+))?(((((/// - Parameters:\s+(/// (( - \w+:)|([^-]{1})).*\s+)+)\s+)|(/// - Returns:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Note:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Throws:.*\s+(/// [^-]{1}.*\s+)*))(///\n\s+)?)+)
  - Regex to match last line of summary section:
```
///[ ]+[^-\n]+\s+?
```
  - Regex to match parameters section:
```
/// - Parameters:\s+
(/// ((  - \w+:)|([^-]{1})).*\s+)+
```
  - Regex to match other section:
```
/// - (Returns|Throws|Note):.*\s+
(/// [^-]{1}.*\s+)*
```
  - Regex to match empty comment line between sections:
```
///\n\s+?
```
- Replacement: $1$2$3$15$7$17$13
  - $1: Last non-empty line of summary section
  - $2: ///
  - $3: New lines and white spaces, use this one and $2 to add empty line between summary and other sections
  - $15: Note section
  - $7: Parameters section
  - $17: Throws section
  - $13: Returns section
Uppercased the section title(regex)
Remove duplicated section(manually)
Fix a document error(manually)

Testing Details 🔍

N/A

codecov-io · 2018-09-04T14:02:21Z

Codecov Report

Merging #3621 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #3621   +/-   ##
=======================================
  Coverage   31.01%   31.01%           
=======================================
  Files         114      114           
  Lines       10482    10482           
=======================================
  Hits         3251     3251           
  Misses       7231     7231

Impacted Files	Coverage Δ
...ta/Implementations/Standard/ScatterChartData.swift	`0% <ø> (ø)`	⬆️
Source/Charts/Charts/BarChartView.swift	`32.5% <ø> (ø)`	⬆️
Source/Charts/Charts/CombinedChartView.swift	`25% <ø> (ø)`	⬆️
Source/Charts/Utils/Platform+Accessibility.swift	`31.57% <ø> (ø)`	⬆️
Source/Charts/Charts/PieRadarChartViewBase.swift	`5.63% <ø> (ø)`	⬆️
...mplementations/Standard/CandleChartDataEntry.swift	`0% <ø> (ø)`	⬆️
Source/Charts/Charts/HorizontalBarChartView.swift	`0% <ø> (ø)`	⬆️
Source/Charts/Charts/ChartViewBase.swift	`19.51% <ø> (ø)`	⬆️
...ource/Charts/Renderers/ChartDataRendererBase.swift	`54.54% <ø> (ø)`	⬆️
Source/Charts/Animation/Animator.swift	`1.61% <ø> (ø)`	⬆️
... and 37 more

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 55028b1...a435669. Read the comment docs.

liuxuan30 · 2018-09-19T07:45:30Z

How did you make this change, manually or by some tools? I cannot trust my eyes reviewing this.. though I tend to be green light to merge

``` (\n[ ]+)(((/// - parameter \w+:.*\s+)(///((\s+)|( \s+.+\s+)))?)*/// - parameter \w+:.*) ``` ``` $1/// - Parameters:$1$2 ```

``` /// - parameter (\w+):(.*)(\s+///(\n))*(\s+) ``` ``` /// - $1:$2$4$5 ```

@IBOutlet

``` /// (note|return|parameters|throws): ``` ``` /// - $1: ```

kemchenj · 2018-09-22T17:00:14Z

@liuxuan30 Basically regex + manually.

I've add some details in the initial comment, include the format guideline and regex that I used.

And I split the changes into separate commits, describe the regex in commit message. I think that would be useful for you guys to review.

``` ((///)[ ]+[^-\n]+(\s+))?(((((/// - Parameters:\s+(/// (( - \w+:)|([^-]{1})).*\s+)+)\s+)|(/// - Returns:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Note:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Throws:.*\s+(/// [^-]{1}.*\s+)*))(///\n\s+)?)+) ``` ``` $1$2$3$15$7$17$13 ```

Source/Charts/Charts/BarChartView.swift

Source/Charts/Charts/ChartViewBase.swift

liuxuan30 · 2018-09-27T01:32:08Z

Source/Charts/Charts/ChartViewBase.swift

@@ -742,46 +775,47 @@ open class ChartViewBase: NSUIView, ChartDataProvider, AnimatorDelegate
    }

    /// *
-    /// - note: (Equivalent of getCenter() in MPAndroidChart, as center is already a standard in iOS that returns the center point relative to superview, and MPAndroidChart returns relative to self)*
-    /// - returns: The center point of the chart (the whole View) in pixels.
+    ///


why we need a /// here?

@kemchenj is this one ok to resolve without any change?

Already removed.

Source/Charts/Data/Implementations/Standard/BarChartDataEntry.swift

liuxuan30 · 2018-09-27T01:35:38Z

Source/Charts/Data/Implementations/Standard/ChartDataSet.swift

@@ -55,8 +55,9 @@ open class ChartDataSet: ChartBaseDataSet
    // MARK: - Data functions and accessors

    /// *
-    /// - note: Calls `notifyDataSetChanged()` after setting a new value.
-    /// - returns: The array of y-values that this DataSet represents.
+    ///


do we need /// here?

Here I regard * as part of "Summary" section. And I am not really sure what it means, maybe it is some magic sign here for someone. So I just leave it here.😂

@jjatie any idea? If it's ok, it seems this PR can be merged now.

The * and the empty /// can be removed.

Ok, I just push a new commit to remove them.

Source/Charts/Data/Implementations/Standard/ChartDataSet.swift

Source/Charts/Data/Interfaces/IChartDataSet.swift

liuxuan30 · 2018-09-27T01:40:08Z

@kemchenj please check above comments. I found some inconsistence of /// usage. I tried to mark every one I found, but please have a double check if I missed some.

Source/Charts/Charts/ChartViewBase.swift

kemchenj · 2018-10-21T12:28:58Z

@liuxuan30 Sorry for late reply. I could not find a proper regex for those inconsistent separator line behavior these days. So I just fixed it manually and I got a double check. Please review it again.

liuxuan30 · 2018-11-01T01:55:38Z

do you forecast there would be more fixes for this PR? Otherwise we could get merged soon

kemchenj · 2018-11-04T12:57:57Z

Nope, I think it won't be more fixes for this PR.

jjatie approved these changes Sep 16, 2018

View reviewed changes

kemchenj force-pushed the master branch 2 times, most recently from dcaf093 to 6de6a78 Compare September 21, 2018 09:27

kemchenj added 4 commits September 22, 2018 21:00

Add Parameters Section

08820aa

``` (\n[ ]+)(((/// - parameter \w+:.*\s+)(///((\s+)|( \s+.+\s+)))?)*/// - parameter \w+:.*) ``` ``` $1/// - Parameters:$1$2 ```

Remove parameter prefix

576d7af

``` /// - parameter (\w+):(.*)(\s+///(\n))*(\s+) ``` ``` /// - $1:$2$4$5 ```

Add missing -

bad3eef

``` /// (note|return|parameters|throws): ``` ``` /// - $1: ```

kemchenj force-pushed the master branch from 6de6a78 to cc1dbcd Compare September 22, 2018 16:23

kemchenj added 4 commits September 23, 2018 09:03

Sort sections

f3fa752

``` ((///)[ ]+[^-\n]+(\s+))?(((((/// - Parameters:\s+(/// (( - \w+:)|([^-]{1})).*\s+)+)\s+)|(/// - Returns:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Note:.*\s+(/// [^-]{1}.*\s+)*)|(/// - Throws:.*\s+(/// [^-]{1}.*\s+)*))(///\n\s+)?)+) ``` ``` $1$2$3$15$7$17$13 ```

Uppercased the section title

0edceea

fix function groupBars document error

c68ed07

Remove duplicated section

033bb11

kemchenj force-pushed the master branch from cc1dbcd to 033bb11 Compare September 23, 2018 01:04

liuxuan30 reviewed Sep 27, 2018

View reviewed changes

Source/Charts/Charts/ChartViewBase.swift Show resolved Hide resolved

Add missing empty line between Summary and other section manually

a4cae71

kemchenj force-pushed the master branch from 15dbb49 to a4cae71 Compare October 21, 2018 12:17

Remove meaningless comment

a435669

liuxuan30 merged commit ce0d809 into ChartsOrg:master Nov 7, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update document to latest format #3621

Update document to latest format #3621

kemchenj commented Sep 4, 2018 •

edited

Loading

codecov-io commented Sep 4, 2018 •

edited

Loading

liuxuan30 commented Sep 19, 2018 •

edited

Loading

kemchenj commented Sep 22, 2018 •

edited

Loading

liuxuan30 Sep 27, 2018

liuxuan30 Oct 24, 2018

kemchenj Oct 28, 2018

liuxuan30 Sep 27, 2018

kemchenj Oct 21, 2018

liuxuan30 Oct 24, 2018

jjatie Oct 27, 2018 •

edited

Loading

kemchenj Oct 28, 2018

liuxuan30 commented Sep 27, 2018

kemchenj commented Oct 21, 2018

liuxuan30 commented Nov 1, 2018

kemchenj commented Nov 4, 2018

Update document to latest format #3621

Update document to latest format #3621

Conversation

kemchenj commented Sep 4, 2018 • edited Loading

Issue Link 🔗

Goals ⚽

Implementation Details 🚧

Testing Details 🔍

codecov-io commented Sep 4, 2018 • edited Loading

Codecov Report

liuxuan30 commented Sep 19, 2018 • edited Loading

kemchenj commented Sep 22, 2018 • edited Loading

liuxuan30 Sep 27, 2018

Choose a reason for hiding this comment

liuxuan30 Oct 24, 2018

Choose a reason for hiding this comment

kemchenj Oct 28, 2018

Choose a reason for hiding this comment

liuxuan30 Sep 27, 2018

Choose a reason for hiding this comment

kemchenj Oct 21, 2018

Choose a reason for hiding this comment

liuxuan30 Oct 24, 2018

Choose a reason for hiding this comment

jjatie Oct 27, 2018 • edited Loading

Choose a reason for hiding this comment

kemchenj Oct 28, 2018

Choose a reason for hiding this comment

liuxuan30 commented Sep 27, 2018

kemchenj commented Oct 21, 2018

liuxuan30 commented Nov 1, 2018

kemchenj commented Nov 4, 2018

kemchenj commented Sep 4, 2018 •

edited

Loading

codecov-io commented Sep 4, 2018 •

edited

Loading

liuxuan30 commented Sep 19, 2018 •

edited

Loading

kemchenj commented Sep 22, 2018 •

edited

Loading

jjatie Oct 27, 2018 •

edited

Loading