Skip to content

Revise doc comments for API reference style. #350

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

Merged
merged 4 commits into from
Apr 27, 2022
Merged

Conversation

amartini51
Copy link
Member

  • Fix spelling error in parameter name (invertion → inversion).
  • Add docs for parameters and return values.
  • Use indicative instead of imperative for method/function/initializer
    abstracts. (Match a regex. → Matches a regex.)
  • Don't use code voice in abstracts, or symbol names as English nouns.
  • Use contractions per Apple style.
  • Turn /// comments that contain notes for this API's implementers into plain // comments, to omit that content from the docs. Move them before the /// comments, to keep doc comments immediately adjacent to the declaration they documents.

- Fix spelling error in parameter name (invertion --> inversion).
- Add docs for parameters and return values.
- Use indicative instead of imperative for method/function/initializer
  abstracts.  (Match a regex. --> Matches a regex.)
- Don't use code voice in abstracts, or symbol names as English nouns.
- Use contractions per Apple style.
- Turn /// comments that contain notes for this API's implementers into
  plain // comments, to omit that content from the docs.  Move them
  before the /// comments, to keep doc comments immediately adjacent
  to the declaration they documents.
@amartini51 amartini51 force-pushed the amartini/docs_91301229 branch from 2ee6aec to ab308ee Compare April 25, 2022 23:54
Copy link
Member

@natecook1000 natecook1000 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, @amartini51!

@milseman milseman requested a review from hamishknight April 26, 2022 13:23
@milseman
Copy link
Member

@hamishknight for regex parser comments

Copy link
Contributor

@hamishknight hamishknight left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_RegexParser changes LGTM, though I do not believe we will be exposing it as public API as it's an implementation-only dependency of _StringProcessing.

Co-authored-by: Nate Cook <[email protected]>
@amartini51
Copy link
Member Author

@swift-ci test

@amartini51
Copy link
Member Author

Looking at the CI builder failures, I don't think they came from this PR's changes, since everything except the spelling change to a parameter name is in comments. @natecook1000 Do you mind taking a look with me?

/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:193:33: warning: result of operator '!=' is unused
    input.wholeMatch(of: regex) != nil
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~
/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:194:10: error: cannot find 'r' in scope
    try? r.regex.wholeMatch(in: self)
         ^
/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:198:11: error: value of type 'Substring' has no member 'wholeMatch'
    input.wholeMatch(of: regex) != nil
    ~~~~~ ^~~~~~~~~~
[150/170] Compiling _StringProcessing Match.swift
/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:193:33: warning: result of operator '!=' is unused
    input.wholeMatch(of: regex) != nil
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~ ^  ~~~
/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:194:10: error: cannot find 'r' in scope
    try? r.regex.wholeMatch(in: self)
         ^
/build/swift-experimental-string-processing/Sources/_StringProcessing/Regex/Match.swift:198:11: error: value of type 'Substring' has no member 'wholeMatch'
    input.wholeMatch(of: regex) != nil
    ~~~~~ ^~~~~~~~~~
[153/170] Emitting module _StringProcessing
/build/swift-experimental-string-processing/Sources/_StringProcessing/PrintAsPattern.swift:21:11: warning: cannot use struct 'AST' in an extension with public or '@usableFromInline' members; '_RegexParser' has been imported as implementation-only
extension AST {
          ^
/build/swift-experimental-string-processing/Sources/_RegexParser/Regex/AST/AST.swift:15:15: note: type declared here
public struct AST: Hashable {
              ^
error: fatalError

Copy link
Member

@milseman milseman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You were adding a line of code here. Could you do a quick pass and make sure it's comments only again?

@milseman
Copy link
Member

@swift-ci please test

Confirmed (using the command below) that this branch now contains only
changes to doc comments, changes to // comments where content was moved
into or out of a doc comment, and one spelling correction to code.

% git diff origin/main... |
    grep -v '^+ *///\|- *///' |
    grep -v '^@@' |
    grep -v '^ ' |
    grep -v '^+++\|^---' |
    grep -v '^diff --git' |
    grep -v '^index '

+  // Individual public API functions are in the generated Variadics.swift file.
+// For now, we use String as the source while prototyping...
+
+  // NOTE: Currently, this means we have raw quotes.
+  // Better would be to have real Swift string delimiter parsing logic.
+
+  // NOTE: traditional comments are not nested. Currently, we are neither.
+  // Traditional comments can't have `)`, not even escaped in them either, we
+  // can. Traditional comments can have `*/` in them, we can't without
+  // escaping. We don't currently do escaping.
+
-  // Finish, flush, and clear. Returns the rendered output
+  // Finish, flush, and clear.
+  //
+  // - Returns: The rendered output.
+  // Note: The `Script` enum includes the "meta" script type "Katakana_Or_Hiragana", which
+  // isn't defined by https://www.unicode.org/Public/UCD/latest/ucd/Scripts.txt,
+  // but is defined by https://www.unicode.org/Public/UCD/latest/ucd/PropertyValueAliases.txt.
+  // We may want to split it out, as it's the only case that is a union of
+  // other script types.
+
+// TODO: These should become aliases for the Block (blk) Unicode character
+// property.
+
-  // Ensures `.0` always refers to the whole match.
-  // Allows `.0` when `Match` is not a tuple.
-  func withInversion(_ invertion: Bool) -> Self {
+  func withInversion(_ inversion: Bool) -> Self {
-    if invertion {
+    if inversion {
@amartini51
Copy link
Member Author

@swift-ci Please test.

@amartini51 amartini51 merged commit 6c5c082 into main Apr 27, 2022
@amartini51 amartini51 deleted the amartini/docs_91301229 branch April 27, 2022 18:31
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.

5 participants